-
Notifications
You must be signed in to change notification settings - Fork 1
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
Initial commit #1
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.
Looking at the PRs I realize that we could do this without the step of changing jcb-gdas. Instead of using a test file you can replace what you have in job-algorithms with:
# Optionally test the application
{% if test_reference_filename is defined %}
test:
reference filename: {{test_reference_filename}}
{% if test_output_filename is defined %}
test output filename: {{test_output_filename}}
{% endif %}
{% if log_output_filename is defined %}
log output filename: {{log_output_filename}}
{% endif %}
{% if mpi_pattern is defined %}
mpi pattern: {{mpi_pattern}}
{% endif %}
float relative tolerance: {{test_float_relative_tolerance | default(1.0e-6, true)}}
float absolute tolerance: {{test_float_absolute_tolerance | default(0.0, true) }}
integer tolerance: {{test_integer_tolerance | default(0, true) }}
{% endif %}
Note that I've accounted for the complete logic here.
Then you only need to change the YAMLs in the GDASapp test directories to include the reference and tolerances. I.e. you need test_reference_filename et al here.
This actually makes more sense because there isn't really a sensible place to add that test file. You correctly put it in the atmosphere directory of jcb-gdas but we'd put the exact same file in the marine, aero etc directories. This way we avoid any changes to jcb-gdas.
Note that we could make it so you don't need to add the above YAML to every file in job-algorithms and instead have a global option to add it to any algorithm but perhaps for later.
@danholdaway Done. Makes sense to keep things more generic, thanks |
…1129) This PR turns on the testing blocks, added in jcb-algorithms #[1](NOAA-EMC/jcb-algorithms#1), in the input YAMLs for the 3dvar and lgetkf jjob test, turning them into JEDI application tests that check for changes in the output states. This PR updates the jcb-algorithms hash to the latest develop as well. The motivation for this PR is that we would like to know not only if changes to GDASApp cause run failures, but also whether they change outputs. I used the same relative and absolute tolerance used in https://github.com/JCSDA-internal/fv3-jedi/blob/develop/test/testinput/lgetkf.yaml and https://github.com/JCSDA-internal/fv3-jedi/blob/develop/test/testinput/hyb-3dvar_gfs.yaml from FV3-JEDI. I don't know why one uses 1e-7 and the other 1e-6 for the absolute tolerances. Reviewers can tell me what they think is reasonable. I ran all the jjob tests and they passed on Orion with the feature/atmensanlfv3inc branch of Global Workflow. That branch needs to be merged into develop with #[2592](NOAA-EMC/global-workflow#2592). Note: The JCB submodule in Global Workflow will need to updated to the latest develop branch due to change in this commit of jcb-algorithms NOAA-EMC/jcb-algorithms@1a1497c
This PR adds a block to the 3dvar and local ensemble DA YAMLs to allow those applications to act as application tests, as long as "test_file" is defined in the JCB prototype YAML. This will be turned on in jjob testing in a future GDASApp PR.