[Mohit Dalvi (mcdalvi)]

PR Summary

Sci/Tech Reviewer: Alan J Hewitt (@alanjhewitt)
Code Reviewer: James Bruten (@james-bruten-mo)

This PR adds two sample diagnostics from the UKCA Chemistry sub-model (OH + CH4 reaction flux and Total Ozone in column).

Note: These fields are already defined outside the STASH framework in UKCA and accessible via its diagnostics API (mosrs:ukca#29) so no modifications are required to ukca repo

Technical Debt The OH+CH4 diagnostic calculation temporarily uses a fixed tropopause level for masking out the stratosphere. Using active/ live tropopause level (which will be essential for some future chemistry diagnostics) will require some pipeline work between LFRic and UKCA.

• linked Remove check for optional diagnostic data array ukca#49
• linked MetOffice/um#137

Code Quality Checklist

• I have performed a self-review of my own code
• My code follows the project's style guidelines
• Comments have been included that aid understanding and enhance the readability of the code
• My changes generate no new warnings
• All automated checks in the CI pipeline have completed successfully

Testing

• I have tested this change locally, using the LFRic Apps rose-stem suite
• If any tests fail (rose-stem or CI) the reason is understood and acceptable (e.g. kgo changes)
• I have added tests to cover new functionality as appropriate (e.g. system tests, unit tests, etc.)
• Any new tests have been assigned an appropriate amount of compute resource and have 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 - apps3.2_sample_chem_diag/run3

Suite Information

Item	Value
Suite Name	apps3.2_sample_chem_diag/run3
Suite User	mohit.dalvi
Workflow Start	2026-09-23T22:35:45
Groups Run	developer

Dependency	Reference	Main Like
casim	MetOffice/casim@2026.07.1	True
jules	MetOffice/jules@2026.07.1	True
lfric_apps	mcdalvi/lfric_apps@apps3.2_sample_chem_diag	False
lfric_core	MetOffice/lfric_core@2026.07.1	True
moci	MetOffice/moci@2026.07.1	True
SimSys_Scripts	MetOffice/SimSys_Scripts@2026.07.1	True
socrates	MetOffice/socrates@2026.07.1	True
socrates-spectral	MetOffice/socrates-spectral@2026.07.1	True
ukca	mcdalvi/ukca@rem_diag_array_check	True

Task Information

✅ succeeded tasks - 1210

Security Considerations

• I have reviewed my changes for potential security issues
• Sensitive data is properly handled (if applicable)
• Authentication and authorisation are properly implemented (if applicable)

Performance Impact

• Performance of the code has been considered and, if applicable, suitable performance measurements have been conducted

AI Assistance and Attribution

• Some of the content of this change has been produced with the assistance 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)

GitHub Copilot via VSCode

Documentation

• Where appropriate I have updated documentation related to this change and confirmed that it builds correctly

PSyclone Approval

• If you have edited any PSyclone-related code (e.g. PSyKAl-lite, Kernel interface, optimisation scripts, LFRic data structure code) then please contact the HPC Optimisation Team

Sci/Tech Review

• I understand this area of code and the changes being added
• The proposed changes correspond to the pull request description
• Documentation is sufficient (do documentation papers need updating)
• Sufficient testing has been completed

(Please alert the code reviewer via a tag when you have approved the SR)

Code Review

• All dependencies have been resolved
• Related Issues have been properly linked and addressed
• CLA compliance has been confirmed
• Code quality standards have been met
• Tests are adequate and have passed
• Documentation is complete and accurate
• Security considerations have been addressed
• Performance impact is acceptable

[Mohit Dalvi (mcdalvi)]

Plots comparing the new diagnostics from a GC6-C64 run (top) vs UKESM1.0-N96 (bottom).

Note: Differences in emissions; time-periods; no tunings applied in GC6.

[Mohit Dalvi (mcdalvi)]

Plots from rose stem test at C12 using Geovista

[iboutle]

Hi Mohit,

I have a slight concern about this, which I'm not sure if can be improved upon...

If I understand correctly, n_ukca_diags_3d is going to be the maximum possible number of diagnostics requestable from UKCA. Currently this is 2, but potentially it could be 100s. The first loop in initialise_ukca_diagnostics has to look over all options to check whether they are requested or not, but subsequent loops (in output_ukca_diagnostics and aerosol_ukca_kernel shouldn't need to perform this entire looping, they should be able to immediately jump to a smaller list of the diagnostics which are actually requested on the current timestep, i.e. a variable n_ukca_diags_req, set by the initial call in initialise_ukca_diagnostics.

The point here is that diagnostic requests are a sparse problem - the vast majority of model runs will not request any ukca diagnostics on the vast majority of timesteps. However, currently we are allocating space for, and checking, all possible diagnostics on every timestep. The do loops probably aren't that much of a problem, but the allocation of diag_fullht_real to be this enormous size every time ukca is called is a worry - the memory overhead of this will be unnecessarily huge.

Is it possible to change this?

A couple of more minor comments:

• is the setting of diag_fullht_real=0 in aerosol_ukca_kernel really necessary - again, this is a costly action on a large array that will happen every timestep regardless of whether it's required or not. It would be better to only do it where necessary, if at all.
• same question for setval_c(rxnflux_oh_ch4_trop, 0.0_r_def) etc in initialise_ukca_diagnostics - I'm fairly sure this is not needed because every element of the array is written to in aerosol_ukca_kernel
• Please ensure the diagnostic map is updated with the stashcode mapping for these new fields.

Thanks!

[Alan J Hewitt (alanjhewitt)]

have a look if case default is needed in a few places and check that "DU" is the approved cf for dobson units please.

[Alan J Hewitt (alanjhewitt)]

Am happy with the code changes. I just need to see the updated trac.log

[Matthew Hambley (MatthewHambley)]

No new global variables please.

[Matthew Hambley (MatthewHambley)]

This file should only ever be removed from. Never added to. Why are there new global variables?

[Mohit Dalvi (mcdalvi)]

Matthew Hambley (@MatthewHambley) A masterlist of all requested diagnostics is built up once at the initialisation stage by querying XIOS, and then whittled down each timestep depending on whether the field is to be output/ sampled in that call.
The alternative will involve defining the field name arrays at two locations (initialisation and algorithm level) and querying XIOS twice on each timestep as the ordering/ indexing of the master vs active list is important.
One option not to increase the dirtylist could be to define these in um_ukca_init_mod which is already in there, although this does not address the issue of global variables itself.
iboutle

[Mohit Dalvi (mcdalvi)]

Thinking of alternatives -could the masterlist be somehow stored in modeldb? (key_value/ linked_list?) It could then be extracted and passed in to aerosol_ukca_alg.

[Mohit Dalvi (mcdalvi)]

I think Copilot has got me partly there using modeldb%values (but not removed all global vairables), so will continue on this and see if it actually works.

[Mohit Dalvi (mcdalvi)]

Matthew Hambley (@MatthewHambley) , iboutle Alan J Hewitt (@alanjhewitt)
I have reworked the change to avoid global variables by trying to use modeldb%values to pass data between initialisation and algorithm levels. This does not seem to support storing of arrays (although the base key_value_collection appears to) so each component of the names array is stored separately. I can open an issue to change this when the array functionality is added in future.
Since there does not seem to be a way to pass character arrays GH_CHAR to the kernel, instead of individual fields a diagnostic super array is being used to obtain the fields from UKCA API and unpacked in the algorithm layer.
I have reused the photol_diags_kernel to extract individual fields from the multi-data field. After release, or on main after #584, this can be converted into a generic kernel routine.

Apologies for sending this back to you so close to the deadline.

[iboutle]

Hi Mohit,

Sorry, I'm just back from leave and catching up on this. I would probably have said to keep the global variables in this instance - as you well know, the UKCA API is designed around these global lists which are setup at initialisation, and used throughout the code. As we have no plans to change this on the UKCA side, it doesn't really seem too problematic for LFRic to accommodate it, as it does with all the other lists.

However, since you've managed to rework it, I'd be inclined to keep what you've done. I like the super array passed into the kernel, as this will avoid having to add new arguments for new diagnostics. I am a bit worried about the computational cost of all the extra work which is having to be done to avoid the global variables, e.g. the extra setup in the algorithm each timestep and copying the data back out of the super array, but we can monitor this and change if necessary.

I've a couple of minor suggestions I'll add to the relevant lines...

[iboutle]

This setting of level 0 is done twice - once here and once in photol_diags_kernel - could one or other be removed?

[Mohit Dalvi (mcdalvi)]

Removed from kernel routine

[iboutle]

Now we pass the super array into the kernel, we don't actually need to allocate the diagnostic fields (rxnflux_oh_ch4_trop) here any more. We could use "diagnostic_to_be_sampled" to establish if it's requested on the current timestep, and then use init_diag to allocate the field inside output_ukca_diagnostics. That would avoid having to plumb the arrays between the initialise and output routines.

[Mohit Dalvi (mcdalvi)]

Thanks that is a handy function to check request status! Fields now local to output_ukca_diagnostics.

[Alan J Hewitt (alanjhewitt)]

I dont want to hold up getting this into code review. When you have time, make sure that the deallocate order is the reverse of the allocate order

[Alan J Hewitt (alanjhewitt)]

Hi Mohit,

The changes are good, thanks for all the hard work.

I dont want to hold this up to get into code review, but it would be worth going back to interfaces/physics_schemes_interface/source/support/ukca_diag_setup_mod.f90 and making sure that the deallocate order is the reverse of the allocate order.

SciTech approved.

Alan

[Matthew Hambley (MatthewHambley)]

Just a drive-by comment. If the only purpose of the chem_scheme variable is to check its value, there should be no need to extract it first. i.e. if ( modeldb$config%chemistry%chem_scheme() == chem_scheme_strattrop ).

This has been addressed