-
Notifications
You must be signed in to change notification settings - Fork 6
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
402 merge from am3 params #404
Conversation
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 for the delay. I forgot to final step of the review process.
The regression testing should happen in benchcab. The user guide is here: https://benchcab.readthedocs.io/en/latest/ I would expect the following config file to do what you need: # Example configuration file for running the CABLE benchmarking.
#
# Note, optional keys are available in this config file. See
# https://benchcab.readthedocs.io/en/stable/user_guide/config_options/
# for documentation on all the available keys.
#
# This file is in YAML format. You can get information on the syntax here:
# https://yaml.org/spec/1.2.2/#chapter-2-language-overview
# Quick tips:
# You need a space after the ":"
#
# It uses the same syntax as Python for:
# - lists (aka sequences)
# - dictionaries (aka mappings with key/value pairs)
#
# Strings can be given with or without double or single quotes.
realisations:
- repo:
git:
branch: main
- repo:
git:
branch: 402-merge-from-am3-params
modules: [
intel-compiler/2021.1.1,
netcdf/4.7.4,
openmpi/4.1.0
] And you don't need to do the analysis on modelevaluation.org. Just check the log file to ensure the regression testing was all successful. |
@ccarouge, I can run benchcab and get results from dozens of runs but the documentation isnt clear WRT what the output is. So what is S0_RO, etc. I have two beaches A and B. How can I see that A is effectively identical to B? |
@JhanSrbinovsky This part in the documentation should tell you where to look:
But I agree, we need to make it clearer where people can find the information about the results in the docs. |
Doh - so you're telling me that the only directory that sounded like what I thought might be relevant was empty by design |
The PBS log file should also have human-friendly logs of success for the regression tests. |
I haven’t looked yet but there has to be an equivalent offline. They could even be hard-wired offline. They were previously hardwired here as well.
|
which is the only place it is used
DOH! - I've got myself in a muddle here. I've started to make changes pushing nCNP_pools index to coupled only, but I have done work in a new issue #412 (+ I have a local branch going) PLUS I have had to make the corresponding changes in AM3 to make sure it works there. Between juggling the kids and a new puppy I've got all these branches mixed up. |
cable_surface_type_namelist - which will be implemented here as well at a later date
…nto 402-merge-from-am3-params
that in grid_constants
BTW - renewed regression testing not only shows no unintended changes in output but absolutely zero changes in output |
# CABLE Thank you for submitting a pull request to the CABLE Project. ## Description Miscellaneous developments in science/ dir. from AM3. These are included separately to more major developments in canopy and soilsnow directories. Fixes #402 ## Type of change Including AM3 development. ## 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 <!-- readthedocs-preview cable start --> ---- 📚 Documentation preview 📚: https://cable--430.org.readthedocs.build/en/430/ <!-- readthedocs-preview cable end -->
@ccarouge - I can't move on this until approved. From what I can tell requested change was made. what else is there? |
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, can't approve. Some changes can't go in. Especially, changes in the radiation would remove a bug fix. We want to keep this fix in main and implement it in ESM1.6, not the other way around.
Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>
@ccarouge - I think I have addressed all the requested changes. It was a few days ago since I last visited this. Can't remember if I re-requested review but that doesnt appear to be an option, suggesting that I have. "see review" doesn't seem to show any others |
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.
One more question. Why are there a bunch of files with the line:
!#define UM_CBL YES
I would have thought files in CABLE offline should have this as NO?
I am using if UM_CBL is defined or not, not what it is defined as. You can also test what it is defined as (I forget the exact syntax) but the test alone is more complicated than just ifdef. As it is switching b/n coupled and not is only 1 character, "!", which by default is in place, i.e. it is NOT defined. The alternative was going to involve two lines, switching between the two would involve commenting the other one, and all of t his for a variation I was hoping would be gone by now, possibly it will soon-ish anyway :). Maybe not b4 Xmas, but in the 1st half of next year. @ccarouge |
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.
Finally! Sorry for the very lengthy delay.
CABLE
Thank you for submitting a pull request to the CABLE Project.
Description
Merge modifications made during in AM3 development.
You can link issues by using a supported keyword in the pull request's description or in a commit message:
Fixes #402
Type of change
Merge modifications made during in AM3 development.
Testing:
benchcab regression testing against
main
show no changes.Checklist
📚 Documentation preview 📚: https://cable--404.org.readthedocs.build/en/404/