-
Notifications
You must be signed in to change notification settings - Fork 8
(#629) - Sync with the work on JULES #630
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
(#629) - Sync with the work on JULES #630
Conversation
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.
All the changes being under JAC I don't really need to read through all the code. We can combine common files later. Hopefully even just submit a library to JULES as the plan was after CMIP5. The plan was rewound back then but I don't think we'll face the same resistance this time. We shall see
| n_soiltypes = 9 ! number of soil types | ||
|
|
||
| REAL, PARAMETER :: & | ||
| max_snow_depth = 50000.0, & ! maximum depth of lying snow on tiles (kg/m2) |
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.
as an aside - this parameter (at least) should be converted to a namelist read since it varies between ESM1.6, CM2, AM/CM3 and offline.
I'm also not sure why some of the dimension setting numbers aren't set in the grid_constants_mod
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.
Opened issue for max_snow_depth (#640).
The rationalisation of the constant files is on going. Only the constants that were used in JAC have been rationalised. So there is a bit of a mix currently (no more than before though).
| INTEGER, PARAMETER :: lakes_cable = 16! SoilType Index (soilparm_cable.nml JAC) | ||
|
|
||
| INTEGER, PARAMETER :: mf = 2 ! # leaves (sunlit, shaded) | ||
| INTEGER, PARAMETER :: niter = 4 ! number of iterations for za/L |
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.
niter shouldn't be being given here and in other_constants_mod - if nothing else you can add to the USE grid_constants_mod line
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.
an error. we would have to check which it even uses. I suspect grid-----. But this is only JULES coupled right, so hopefully not in all models
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.
Since it's JAC we won't ever have got to the point where the potential for conflict is an issue.
Most of what is in grid_constants and other_constants is common across all apps - so ideally these MODULES would be in the library. However for compilation purposes we'll need a version of these in JULES I think [I'm getting lost what's where now]
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.
@har917 all CABLE constants will come from the library. For JULES currently (i.e. before the compilation is worked out), the constants are hard-coded in the JULES files with a comment to change when we work on the compilation.
Since these constant files are not currently used, I'll remove the niter declaration in grid_constants_cbl.F90 (because niter is not a constant defining the grid).
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.
Sorry, just realised niter is currently (and annoyingly) a grid constant. So should be in grid_constants for now.
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.
Two changes needed I think
niteris defined twice- number of carbon stores
nsCsshould surely be 2 (INH edit on 27/10/2025 - this was supposed to readnsCSshould surely be 3 - but I'm now leaning towards 2 being correct and being mistaken earlier).
@JhanSrbinovsky please note point 2. I am completely confused here. Yes, there is a difference between the CASA variables and the bgc% variables - but the luc3 code (which is based on CASA) also appears to use nsCs, as does the sli code.
Basically worth checking - and there is a longer-term question around whether to maintain the older bgc% code for carbon cycle work.
|
dec of nsCs is only once: dec of niter is only once: IN CABLE:main, ESM1.6. I thought (if not already the same) @ccarouge 's plan was that we can sort this out later when we re-engage with JULES? |
708b83d to
fb10e18
Compare
|
Ready for review @har917 . I would think the |
har917
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.
okay by me.
As an aside - we likely need an exercise to update/correct the FORD documentation in light of all the the code re-organisation. I suspect that some of the inline links are out of date
CABLE
Thank you for submitting a pull request to the CABLE Project.
Description
Fixes #629
This goes with the work in JULES to remove the CABLE files from JULES for the licence change.
The changes are as follow:
spitter()was present but correspond to relatively new work in CABLE so I have chosen to accept the new version from CABLE. See notes:sfactparameters have been given names instead of being hard-coded directly within the algorithmspitter()bug (bug in spitter call (MAIN branch) #355) . Original lines for the radiation split in JAC:Two parameter files show differences between JULES and CABLE and need to be brought in sync in future work. These are copied under JAC/params/cable
Type of change
Please delete options that are not relevant.
Checklist
Testing
The changes do not impact offline CABLE at all. They are linked to the JAC project and will only be used once we work on the new compilation to link JULES and CABLE.
Please add a reviewer when ready for review.
📚 Documentation preview 📚: https://cable--630.org.readthedocs.build/en/630/