Align adjoint_tests to linear_model

[tom-j-h]

PR Summary

Sci/Tech Reviewer:
Code Reviewer: @TeranIvy

Align default configuration of adjoint_tests to that of linear_model (nwp-gal9). This includes both the canned test and the rose-stem tests.

linear_model uses multigrid preconditioning in the solver, and taking advantage of this in the adjoint requires a minor code change in science/adjoint/source/algorithm/solver/adj_semi_implicit_solver_alg_mod.x90, and to test this, minor code changes in some of the test algorithms in applications/adjoint_tests/source/algorithm/solver/

linear_model also reads linearisation states from file, whereas adjoint_tests currently sets them up analytically. Moving to reading from file requires some minor changes e.g., to iodef files, but also allows the removal of a lot of the analytical linearisation state setup code in some of the test algorithms.

In order to keep the alignment of adjoint_tests with linear_model simple, any necessary deviations are handled by optional configurations, so that the base configurations (rose-app.conf files) always remain identical.

This work has highlighted a minor bug which can also be dealt with by way of an optional configuration. I have opened #87 for this. Once it is dealt with the optional configuration will simply need to be removed.

For now, adding C224 tests to adjoint_tests, to match those in linear_model, is not being done, due to ongoing issues with running adjoint_tests in parallel (see https://code.metoffice.gov.uk/trac/lfric_apps/ticket/800).

Finally, the code/patch changes from #71 (science/adjoint/source/kernel/transport/mol/atl_poly1d_vert_w3_reconstruction_kernel_mod.F90 and science/adjoint/patches/kernel/atl_poly1d_vert_adv_kernel_mod.patch) are required for one of the configuration changes, so they've been duplicated here but need not be reviewed again. This makes #71 a blocker. Done!

Also please note I branched from main rather than stable by mistake - apologies for this. I can make a new branch/PR if desired.

• is blocked-by Fixing adjoint failures with transport log_space config variable set to true #71
• closes Align adjoint_tests to linear_model #84
• is related to Align jedi_lfric_tests linear model/adjoint testing to adjoint_tests/linear_model #85
• is related to Incorrect adjoint semi-implicit timestep when reference_reset_time != dt #87

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 Core rose-stem suite
• If required (e.g. API changes) I have also run the LFRic Apps test suite using this branch
• 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

Very long - see ~tom.hill/cylc-run/align_adjoint_tests_to_linear_model-developer-postSR1/trac.log.

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)

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 TCD 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

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

[DrTVockerodtMO]

Thanks Tom. Changes look fine so far (I checked the trac.log and I can confirm the test failure is a test that's unrelated to the changes here).

I've suggested a few minor changes, and I have some questions as well about the adjoint test parameter changes.

[DrTVockerodtMO]

Just to keep things tidy and as fast as possible - the adj_pressure_operator_type ctor can be removed, as the create_adj_pressure_preconditioner method features this line.

[DrTVockerodtMO]

Ditto with this line.

[DrTVockerodtMO]

Ditto.

[DrTVockerodtMO]

What is the reason that the lower range values needed changing? Is it to avoid an LS value of zero because of bugs, or because physically speaking these LS values cannot go to zero?

I think I got the initial values by looking at the initial analytical state fed into the linear_model app, so you may be correct in changing these, I am just curious as to the reason!

[tom-j-h]

Thanks for spotting this, this change shouldn't be here.

It's leftover from when I was getting adjoint_tests to run with all the code built at 32-bit. This caused some tests to occasionally fail with infinities. Making the changes you see here stopped that. From looking at where the failures were, my suspicion was that the issue wasn't divide-by-zero, rather divide-by-tiny-number which, at 32-bit, could more easily cause an infinity than at 64-bit. Hence removing the possibility of numbers in the range [0,0.1] fixed it.

I suggest just removing this change, since it's not needed for the 64-bit tests. The alternative is to keep it, and add the 32-bit builds/runs to the test suite. A caveat is that this does require increasing tolerances slightly for some of the tests involving the iterative solver, as might be expected.

What do you think?

[DrTVockerodtMO]

Thank you for letting me know. I think it's fine to remove the changes for now, and if/when we want 32-bit tests we can introduce it there to keep things agile.

[DrTVockerodtMO]

Thanks for making the changes - looks good to me now!