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

402 merge from am3 params #404

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

JhanSrbinovsky
Copy link
Collaborator

@JhanSrbinovsky JhanSrbinovsky commented Sep 17, 2024

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.

Checklist

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

📚 Documentation preview 📚: https://cable--404.org.readthedocs.build/en/404/

@JhanSrbinovsky JhanSrbinovsky linked an issue Sep 17, 2024 that may be closed by this pull request
Copy link
Collaborator

@ccarouge ccarouge left a 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.

src/params/grid_constants_cbl.F90 Show resolved Hide resolved
src/params/grid_constants_cbl.F90 Outdated Show resolved Hide resolved
@ccarouge
Copy link
Collaborator

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.

@JhanSrbinovsky
Copy link
Collaborator Author

JhanSrbinovsky commented Sep 23, 2024

@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?

@ccarouge
Copy link
Collaborator

@JhanSrbinovsky This part in the documentation should tell you where to look:

runs/fluxsite/analysis/bitwise-comparisons
directory that contains the standard output produced by the bitwise comparison command: benchcab fluxsite-bitwise-cmp. Standard output is only saved when the netcdf files being compared differ from each other

But I agree, we need to make it clearer where people can find the information about the results in the docs.

@JhanSrbinovsky
Copy link
Collaborator Author

Doh - so you're telling me that the only directory that sounded like what I thought might be relevant was empty by design

@ccarouge
Copy link
Collaborator

The PBS log file should also have human-friendly logs of success for the regression tests.

@JhanSrbinovsky
Copy link
Collaborator Author

JhanSrbinovsky commented Sep 24, 2024 via email

@JhanSrbinovsky
Copy link
Collaborator Author

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
@JhanSrbinovsky JhanSrbinovsky reopened this Oct 7, 2024
@JhanSrbinovsky JhanSrbinovsky self-assigned this Oct 7, 2024
@JhanSrbinovsky JhanSrbinovsky added the priority:high High priority issues that should be included in the next release. label Oct 7, 2024
@JhanSrbinovsky JhanSrbinovsky added this to the ESM1.6 FastTrack milestone Oct 7, 2024
@JhanSrbinovsky
Copy link
Collaborator Author

BTW - renewed regression testing not only shows no unintended changes in output but absolutely zero changes in output

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:high High priority issues that should be included in the next release.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Merge from AM3 - params
2 participants