Remove module scope access of namelist variables by coordinate/native jacobian.#249
Remove module scope access of namelist variables by coordinate/native jacobian.#249mo-rickywong wants to merge 8 commits intoMetOffice:mainfrom
Conversation
…ve jacobian in preparation for namelist api port
mo-lucy-gordon
left a comment
There was a problem hiding this comment.
Overall this looks good but I think there are a few duplicated typos - see the code comments.
mo-lucy-gordon
left a comment
There was a problem hiding this comment.
Converted to some suggested typo changes. As these are only simple changes I will pass them to Andy to fix so we can get the tickets committed, as Ricky is away.
components/science/source/kernel/geometry/sci_coordinate_jacobian_mod.F90
Outdated
Show resolved
Hide resolved
components/science/source/kernel/geometry/sci_coordinate_jacobian_mod.F90
Outdated
Show resolved
Hide resolved
components/science/source/kernel/geometry/sci_coordinate_jacobian_mod.F90
Outdated
Show resolved
Hide resolved
components/science/source/kernel/geometry/sci_native_jacobian_mod.F90
Outdated
Show resolved
Hide resolved
Co-authored-by: mo-lucy-gordon <120176477+mo-lucy-gordon@users.noreply.github.com>
|
Your CLA signature was found on the base branch, but you appear to have modified the CONTRIBUTORS.md file in this PR. Please do not edit the CONTRIBUTORS.md file. If you have already signed the CLA, revert changes to the file and your signature will be picked up. |
|
I've applied the small number of spelling corrections suggested by review as @mo-rickywong is away and they are very minor changes. |
| topology_fully_periodic | ||
| use finite_element_config_mod, only: coord_system_xyz, & | ||
| coord_system_native | ||
| ! use planet_config_mod, only: scaled_radius |
There was a problem hiding this comment.
Should this commented out line be left here?
| north_pole(1) = PI/2.0_r_def | ||
| north_pole(2) = 0.0_r_def | ||
| call init_chi_transforms(north_pole_arg=north_pole) | ||
| call init_chi_transforms(geometry_spherical,& |
There was a problem hiding this comment.
| call init_chi_transforms(geometry_spherical,& | |
| call init_chi_transforms(geometry_spherical, & |
| north_pole(2) = 0.0_r_def | ||
| call init_chi_transforms(north_pole_arg=north_pole) | ||
| call init_chi_transforms(geometry_spherical,& | ||
| topology, & |
There was a problem hiding this comment.
| topology, & | |
| topology, & |
| feign_finite_element_config, & | ||
| feign_planet_config | ||
|
|
||
| !use feign_config_mod, only : feign_base_mesh_config, & |
There was a problem hiding this comment.
Should this be removed instead of being commented out?
| call feign_planet_config( scaling_factor=1.0_r_def ) | ||
|
|
||
| call init_chi_transforms() | ||
| !call feign_extrusion_config( method=method_uniform, & |
There was a problem hiding this comment.
Should this be removed instead of being commented out?
| pointwise_coordinate_jacobian, & | ||
| pointwise_coordinate_jacobian_inverse | ||
|
|
||
| ! use base_mesh_config_mod, only : geometry_spherical, & |
There was a problem hiding this comment.
Should this be removed instead of being commented out?
|
Apologies for sticking my oar in, I just got notified about this PR. I realise it's a technical change rather than a science one, but I'm a bit surprised it didn't need a sci/tech reviewer since it changes the API to an important science routine. I understand the logic for this change though, so have no objections but have spotted some things that need tidying. |
I presumed the comments were deliberate but Ricky is away right now so we can't ask him. |
PR Summary
Sci/Tech Reviewer:
Code Reviewer: @mo-lucy-gordon
As part of work to remove configuration namelist access from module scope, configuration variables will need to be passed
by argument. Configuration variables passed by argument to kernels is more involved, so this PR moves the use _config_mod from the Jacobian routines up to the to the kernel level in preparation of further changes.
The main changes is that namelist configuration variables:
Are to be passed by argument jacobian routines, i.e.
The knock-on of this is a large number of kernels/unit-tests need to be updated in
lfric_coreandlfric_apps. Aswell as a change to the arguments ofinit_chi_tranformscalled in the unit_tests.Similar changes, but to a large number of files.
Linked PRs
Code Quality Checklist
Testing
Rose test-suite Runs Green
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review