Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LFRic] [PSyAD] PSycloned adjoint tests add too many basis arguments in adjoint code call #2335

Open
DrTVockerodtMO opened this issue Sep 27, 2023 · 4 comments
Assignees
Labels
adjoint bug Release Planning and creating PSyclone releases

Comments

@DrTVockerodtMO
Copy link
Collaborator

Following the merge of #2233, the psy file for the generated adjoint test of some kernels is incorrect.

E.g.: For sample_eos_pressure in LFRic, the tl code call in the psy file looks like
CALL tl_sample_eos_pressure_code(nlayers, exner_proxy%data, rho_proxy%data, theta_proxy%data, moist_dyn_gas_proxy%data, & &ls_rho_proxy%data, ls_theta_proxy%data, ls_moist_dyn_gas_proxy%data, ndf_w3, undf_w3, map_w3(:,cell), basis_w3_on_w3, ndf_wtheta, & &undf_wtheta, map_wtheta(:,cell), basis_wtheta_on_w3)

This is a valid call signature. But for the adjoint we have
CALL adj_sample_eos_pressure_code(nlayers, exner_proxy%data, rho_proxy%data, theta_proxy%data, moist_dyn_gas_proxy%data, & &ls_rho_proxy%data, ls_theta_proxy%data, ls_moist_dyn_gas_proxy%data, ndf_w3, undf_w3, map_w3(:,cell), basis_w3_on_w3, & &basis_w3_on_wtheta, ndf_wtheta, undf_wtheta, map_wtheta(:,cell), basis_wtheta_on_w3, basis_wtheta_on_wtheta)

The adjoint code call has extra basis arguments which causes build errors within LFRic.

@DrTVockerodtMO DrTVockerodtMO changed the title [LFRic] [PSyAD] Generated adjoint tests add too many basis arguments in adjoint code call [LFRic] [PSyAD] PSycloned adjoint tests add too many basis arguments in adjoint code call Sep 27, 2023
@TeranIvy TeranIvy added Release Planning and creating PSyclone releases adjoint labels Sep 27, 2023
@TeranIvy TeranIvy mentioned this issue Sep 27, 2023
12 tasks
@arporter
Copy link
Member

I suspect this is because we now update the metadata of the adjoint kernel and the fact that it writes to fields that weren't previously written to (and are on spaces for which it requires basis/diff-basis) means that it now expects more arguments. Obviously, we haven't handled that in the transformed kernel itself though which is a bug.
This might be non-trivial to fix!

@arporter arporter added the bug label Sep 27, 2023
@arporter arporter self-assigned this Sep 27, 2023
@arporter arporter reopened this Sep 27, 2023
@rupertford rupertford self-assigned this Sep 27, 2023
@rupertford
Copy link
Collaborator

I think @arporter has summarised the issue well but I'll repeat it in my words. The LFRic harness code that PSyAD is able to generate calls the tl and adj versions of kernels (sample_eos_pressure in this case) from within invokes. When PSyclone is run on this code the associated kernel metadata is examined and the the argument list as expected by the kernel metadata is created. As we are now modifying the kernel metadata (for adjoint code) the expected argument list can change. However, at the moment only the kernel metadata is modified so we can end up with an invalid kernel argument list. The way to solve this is to update the kernel argument list with the corrected arguments. The information to do this should be possible e.g. by comparing TL and ADJ argument lists, but could benefit from LFRic kernel raising so we better know what the arguments are - but this would require some additional work.

@rupertford
Copy link
Collaborator

Created branch 2335_incorrect_adj_args to try to solve this problem.

@rupertford
Copy link
Collaborator

The determination of arguments from metadata using the new approach is blocked by #2079, so that would need to be implemented first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adjoint bug Release Planning and creating PSyclone releases
Projects
None yet
Development

No branches or pull requests

4 participants