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

#ifdef okay for now - but surely (for this one) we can get round the need for this by modifying the files names in the offline/coupled codebases to be the same. #436

Closed
JhanSrbinovsky opened this issue Oct 28, 2024 · 1 comment · Fixed by #446
Assignees

Comments

@JhanSrbinovsky
Copy link
Collaborator

          `#ifdef` okay for now - but surely (for this one) we can get round the need for this by modifying the files names in the offline/coupled codebases to be the same.

Originally posted by @har917 in #430 (comment)

I thought I had raised an issue about this, but at a glance it doesnt look like it. Yes change names, as first step, ultimately read surface_types from namelist as in JAC, but as with other use as well - offline doesnt have access to same JULES checks/balances, as well as other stuff. Should at least combine *2 coupled versions and sit with a coupled and an offline version of namelist reader.

@JhanSrbinovsky
Copy link
Collaborator Author

Some further background - title reflect issue raised by @har917 in review of AM3 - CABLE reconciliation. AM3 (partially at least) follows JAC, reading surface_types from a namelist. surface types (note not Fortran TYPEs - integer indexes) are declared in cable_surface_types.F90. AM3/JAC has its own version of this file and hence USEs cable_surface_types throughout. offline does/did not have this and so USEs a hard-wired version in grid_constants_cbl. From memory Ramzi just added one for offline. In porting back AM3 version of CABLE, there were several instances where it USEd cable_surface_types for even more surface types. i.e. EGBL (EverGreen BroadLeaf), EGNL etc. The quickest work-arounds was to just add hard-wired versions alongside the others in grid_constants. Even without these extra types, we still have an IF AM3 - get the type index from cable_surface_types, else if offline get the type index from grid_constants. Being in the header we have to use FPP directives (#ifdef). FPP directives are not popular with the UKMO and possibly not even legal in LFRic. Hence Ians "objection" is a valid concern.

The same module name can indeed be USEd as there are seperate versions picked up by coupled/offline apps. However some additional plumbing needs to be tweaked offline.

I'll submit the PR so you have time to look at it. I dont expect any difference at all, there are no science changes, however a 5-site benchcab test follows:

@JhanSrbinovsky JhanSrbinovsky linked a pull request Oct 30, 2024 that will close this issue
3 tasks
@JhanSrbinovsky JhanSrbinovsky self-assigned this Nov 18, 2024
JhanSrbinovsky added a commit that referenced this issue Nov 24, 2024
# CABLE

The same module name can indeed be USEd as there are seperate versions
picked up by coupled/offline apps. However some additional plumbing
needs to be tweaked offline.

Please see issue for further details:
Fixes #436
## Type of change

## Checklist

- [x] The new content is accessible and located in the appropriate
section.
- [x] I have checked that links are valid and point to the intended
content.
- [x] I have checked my code/text and corrected any misspellings

Please add a reviewer when ready for review.


<!-- readthedocs-preview cable start -->
----
📚 Documentation preview 📚:
https://cable--446.org.readthedocs.build/en/446/

<!-- readthedocs-preview cable end -->

---------

Co-authored-by: Ben Schroeter <[email protected]>
Co-authored-by: Claire Carouge <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant