-
Notifications
You must be signed in to change notification settings - Fork 8
Merge phenvariable module into casavariable #676
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
base: main
Are you sure you want to change the base?
Conversation
| src/science/casa-cnp/casa_inout.F90 | ||
| src/science/casa-cnp/casa_param.F90 | ||
| src/science/casa-cnp/casa_phenology.F90 | ||
| #src/science/casa-cnp/casa_phenology.F90 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just kill this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I think (and I forgot to mention this) I think it will break ESM1.6, when we try to update the CABLE code. There are a number of mentions of phenvariable in the UM7 code- I kept this here for now as a reminder, mainly to myself, that this needs to be addressed in UM7 as well.
JhanSrbinovsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do like having a separate module for phenology parameters, in principal phen parameters could be used in the absence of CASA. My bigger concern is that this renders the CABLE in CABLE:main different to that in ESM1.6?
I can take this avenue as well, it would just require changes on the JULES side, rather than the CABLE side. We either split the phenology type definitions and allocations out of |
|
We are far away from incorporating CASA into a JAC framework. This whole AM3 version was in the direction to accomplish that. However, I think that now it might be more prudent to proceed in the direction of staying closer to the ESM1.6 version of CABLE/CASA? Typically what happens is we do a CMIP using CABLE version X and then for the next 5+ years all research stems from CMIP using CABLE-X. Variation on the code level are much less common than changes in configuration, requested output - and thats if they even run the model again. Probably most common is they just use the CMIP data. In the meantime CABLE will evolve. I suppose should this be relative to a common CMIP7 version of CABLE? or the ESM/AM3 version? The version of CABLE roughly corresponding to the science/ directory, or what has been included in the library. You have probably noticed that the directory structure of CABLE follows that of JULES. This was a CABLE 3 thing that made the mapping trivial. At that time the UMST were adamant that the code be present in their repo if it was intended to ship with JULES releases. They seem to have changed their position on this. Prior to CABLE 3 there was We "basically" pulled things out of core/ to create science/ utils/ params/ etc. coupled/ contained a further breakdown of e.g. ESM/ CM2/. In summary, perhaps WRT to the changes in science/ we should keep them as in ESM1.6. Perhaps stash the AM3 style changes into a branch that we can pick up later if necessary? |
|
I'll do up a PR containing the set of JULES changes required to achieve the same goal as this PR, which is to be able to use the CABLE |
CABLE
Thank you for submitting a pull request to the CABLE Project.
Description
Merges the
src/science/casa-cnp/casa_phenology.F90file intosrc/science/casa-cnp/casa_variable.F90. That file already defines the rest of the CASA derived types- there's no reason for it not to define this as well. Also makes the merging with AM3 easier, as it's version ofcasa_variable.F90and the offline version will have a unified purpose.Fixes #674
Type of change
Testing
Are the changes bitwise-compatible with the main branch? If working on an optional feature, are the results bitwise-compatible when this feature is off? If yes, copy benchcab output showing successful completion of the bitwise compatibility tests or equivalent results below this line.
Are the changes non bitwise-compatible with the main branch because of a bug fix or a feature being newly implemented or improved? If yes, add the link to the modelevaluation.org analysis versus the main branch or equivalent results below this line.
Please add a reviewer when ready for review.
📚 Documentation preview 📚: https://cable--676.org.readthedocs.build/en/676/