Update UKCA initialisation for dust only to include segment size #83
Conversation
github-actions
bot
added
cla-signed
alanjhewitt
left a comment
alanjhewitt
left a comment
There was a problem hiding this comment.
Hi Oakley,
Thanks for making this fix. As mentioned offline, Jon Elsey did not have the ability to test dust_and_clim on the HPC platform available to Leeds, so this slipped through the cracks.
Am also happy with the trailing whitespace being removed.
Paul Earnshaw has no objections to the test being renamed.
!SciTech Approved
Alan
| assert sr.returncode == 0, sr.stderr.decode("UTF-8") | ||
|
|
||
| def test_run_lfric_atm_nwp_gal9_mgnoukca_C48_MG_ex1a_cce_fast_debug_64bit(self, monkeypatch): | ||
| def test_run_lfric_atm_nwp_gal9_mg_C48_MG_ex1a_cce_fast_debug_64bit(self, monkeypatch): |
There was a problem hiding this comment.
Do you need to get approval from the owner of the test to change the name? I will check with Paul ernshaw about this, as he is likely the owner of the test.
There was a problem hiding this comment.
I spoke to Paul. its not a problem as its a technical change. He would want to see if there was a science change.
| ! General GLOMAP configuration options | ||
| ! | ||
| i_mode_nzts=15, & | ||
| ukca_mode_seg_size=i_ukca_mode_seg_size, & |
There was a problem hiding this comment.
This change will make dust_and_clim work correctly with the changes that Jon Elsey made last year.
Including UKCA diagnostics and UKCA namelist items in LFRic is now close to the top of my todo list. This module is technical debt until I can replace the whole thing with namelist settings.
Thanks Alan, can you 'Approve' the PR through the green 'Review' button at the top of this PR please? |
… when intialising UKCA arrays
|
@alanjhewitt would you mind reviewing the additional changes I have made please? |
2 participants
PR Summary
Sci/Tech Reviewer: @alanjhewitt
Code Reviewer: @cameronbateman-mo
The
lfric_atmmodel currently relies on thenoukcaconfiguration to be able to run with OMP_NUM_THREADS > 1. When using a more standard configuration (e.g. um_dump) the runs fail, citing problems inukca_aero_ctl.F90.After investigation in #73, the problem was that UKCA segment size was not passed to the initialisation of UKCA when using
dust_and_climsettings. This is a small fix that mirrors the solution inukca_init()inum_ukca_init_mod.f90.I also removed the
noukcaconfiguration from the small threaded tests (C48_MG, C192_MG) since these were only made to get around the problem fixed in this PR.Warning
This means that the old KGOs must be removed and new ones added for these tests.
I have tested C896_MG and tests >1 thread pass and do not require the
noukcaconfiguration. I tested with both GNU and CCE at fast-debug.Code Quality Checklist
(Some checks are automatically carried out via the CI pipeline)
style guidelines
readability of the code
Testing
acceptable (eg. kgo changes)
tests, unit tests, etc.)
and have tests been allocated to an appropriate testing group (i.e. the
developer tests are for jobs which use a small amount of compute resource
and complete in a matter of minutes)
trac.log
Test Suite Results - lfric_apps - lfric-multithread/run1
Suite Information
Task Information
❌ failed tasks - 6
✅ succeeded tasks - 1450
⌛ waiting tasks - 1
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
PSyclone Approval
inteface, optimisation scripts, LFRic data structure code) then please
contact the
tooscollabdevteam@metoffice.gov.uk
Sci/Tech Review
Please alert the code reviewer via a tag when you have approved the SR
Code Review