Repository navigation
3D chunking for chemistry full-domain mode - #36
Conversation
The new `ukca_config%ukca_asad_full_chunk_size` parameter controls the 3D chunk size. For example, ``` ukca_config%ukca_asad_full_chunk_size(:) = [8, 4, 2] ``` leads to a chunk size of 8x4x2, i.e. 8 elements per row, 4 rows, and 2 levels. A value <= 0 in any dimension will lead to a chunk size equal to the full-domain size in that dimension.
7b45b8d to
783ba21
Compare
|
Mohit Dalvi (@mcdalvi) Might you be SciTech reviewer for this one as you've looked at our chemsitry chunking work in the past? |
Joe Wallwork (joewallwork)
left a comment
There was a problem hiding this comment.
Hi mn416, this looks great! I have a couple of minor comments.
|
Thanks Joe Wallwork (@joewallwork), I've addressed your comments in my latest commit. I've also linked my new |
Joe Wallwork (joewallwork)
left a comment
There was a problem hiding this comment.
Thanks mn416, I'm now happy to approve Sci-Tech review.
| full_chunk_x = row_length | ||
| IF (full_chunk_y <= 0 .OR. full_chunk_y > rows) full_chunk_y = rows | ||
| IF (full_chunk_z <= 0 .OR. full_chunk_z > model_levels) & | ||
| full_chunk_z = model_levels |
There was a problem hiding this comment.
I would suggest printing a warning here within each IF to say that the value of the chunk has been reset to row_length / rows / model_levels, otherwise the code is changing something without the user being informed. The metadata for this namelist entry does say that this will be happening, but I'd also put a warning message here that it's been done
There was a problem hiding this comment.
Thanks Luke Abraham (@theabro), done in commit 1304838.
|
Pierre Siddall (@Pierre-siddall) this has passed sci-tech review |
Luke Abraham (theabro)
left a comment
There was a problem hiding this comment.
Thanks Matt - I'm happy to approve this as code owner
Pierre Siddall (Pierre-siddall)
left a comment
There was a problem hiding this comment.
Thanks mn416 this looks almost good to be approved to me. I've just found one really trivial styling issue and then I'll be happy to put this through testing.
| IF (ukca_config%l_ukca_asad_full) THEN | ||
| ukca_config%ukca_chem_full_chunk_size(:) = [-1, -1, -1] | ||
| IF (PRESENT(ukca_chem_full_chunk_size)) & | ||
| ukca_config%ukca_chem_full_chunk_size(:) = ukca_chem_full_chunk_size(:) | ||
| END IF |
There was a problem hiding this comment.
If an END IF and THEN statement could be added here to keep styling consistent that would be great.
| IF (ukca_config%l_ukca_asad_full) THEN | |
| ukca_config%ukca_chem_full_chunk_size(:) = [-1, -1, -1] | |
| IF (PRESENT(ukca_chem_full_chunk_size)) & | |
| ukca_config%ukca_chem_full_chunk_size(:) = ukca_chem_full_chunk_size(:) | |
| END IF | |
| IF (ukca_config%l_ukca_asad_full) THEN | |
| ukca_config%ukca_chem_full_chunk_size(:) = [-1, -1, -1] | |
| IF (PRESENT(ukca_chem_full_chunk_size)) THEN & | |
| ukca_config%ukca_chem_full_chunk_size(:) = ukca_chem_full_chunk_size(:) | |
| END IF | |
| END IF |
There was a problem hiding this comment.
Thanks Pierre Siddall (@Pierre-siddall), done in commit c78ab29.
There was a problem hiding this comment.
Thanks Pierre Siddall (Pierre Siddall (@Pierre-siddall)), done in commit c78ab29.
No problem mn416 and thanks for responding to the feedback quickly.
Pierre Siddall (Pierre-siddall)
left a comment
There was a problem hiding this comment.
I'm happy that all comments made in my previous review have been addressed, thanks again mn416 for doing this. This is now approved and heading into the queue for testing.
b86cab3
into
MetOffice:main
PR Summary
Sci/Tech Reviewer: Joe Wallwork (@joewallwork)
Code Reviewer: Pierre Siddall (@Pierre-siddall)
UKCA provides various ways to invoke the chemistry solver: level-wise mode (with 1D chunking), column mode (with 1D chunking), and full-domain mode. This PR introduces 3D chunking for full-domain mode, generalising and subsuming all previous modes of operation while enabling new kinds of chunking that were not expressible before. For example, the new namelist entry
(assuming
l_ukca_asad_full=.true.) specifies a chunk size of8x4x2, i.e. 8 grid points per row, 4 rows, and 2 levels. A value of -1 in any dimension will lead to a chunk size equal to the full-domain size in that dimension. This extra flexibility leads to performance improvements both with and without OpenMP enabled, as it is better able to exploit the spatial proximity of hard-to-solve grid points. Here are results comparing full-domain chunking (with a chunk size of8x8x2) against the default UKCA configuration (column mode with a chunk size of a single column).Timings are of the form T_first + T_rest = T_all where T_first, T_rest, and T_all are the times taken for the first two UM time steps, the subsequent time steps, and all time steps respectively.
In summary, 3D chunking gives an overall improvement of 7.4-16.0%, broken down into a 17.8-39.5% improvement for the first two time steps, and a 3.8-4.7% improvement for subsequent time steps.
Code Quality Checklist
(Some checks are automatically carried out via the CI pipeline)
readability of the code
Testing
stem suites
acceptable (eg. kgo changes)
tests, unit tests, etc.)
The UM has been run locally on the N48 configuration for 1 MPI task and 6 time steps, with the following main results from
mule-cumf:Running full-domain mode with a chunk size equal to the full domain gives a perfect match when compared against the existing full-domain mode without chunking.
Running full-domain mode with a chunk size of
1x1x-1, i.e. 1 column, gives a perfect match when compared against column mode with a chunk size of a single column.The UM has been run locally with various values for the new namelist parameter, including: (1) the use of -1 to signify no chunking in particular dimension; (2) a chunk size dimension that exceeds the full-domain size of a dimension, in which case the chunk size is truncated; (3) a chunk size that divides perfectly into the full domain size; and (3) a chunk size that does not divide perfectly into the full domain size, in which case the chunk size in non uniform. In all cases, chunking is performed as intended.
trac.log
Testing branch: https://github.com/RobWatersMet/um/tree/testing-mn416-3d-chunking
Macro has been applied and
dependencies.shupdated with related UKCA branchTest Suite Results - um - testing_mn416_3d_chunking/run1
Suite Information
Approvals
Code Owners
Config Owners
No UM Config Owners Required
Task Information
✅ succeeded tasks - 2770
Security Considerations
Performance Impact
performance measurements have been conducted
AI Assistance and Attribution
of Generative AI tool name (e.g., Met Office Github Copilot Enterprise,
Github Copilot Personal, ChatGPT GPT-4, etc) and I have followed the
Simulation Systems AI policy
(including attribution labels)
Documentation
confirmed that it builds correctly
Sci/Tech Review
Please alert the code reviewer via a tag when you have approved the SR
Code Review