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

Initial commit #1

Merged
merged 6 commits into from
May 24, 2024
Merged

Initial commit #1

merged 6 commits into from
May 24, 2024

Conversation

DavidNew-NOAA
Copy link
Contributor

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.

3dvar.yaml.j2 Outdated Show resolved Hide resolved
Copy link
Collaborator

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

@DavidNew-NOAA
Copy link
Contributor Author

@danholdaway Done. Makes sense to keep things more generic, thanks

@danholdaway danholdaway merged commit df03b11 into develop May 24, 2024
@danholdaway danholdaway deleted the feature/jjob-test branch May 24, 2024 18:39
CoryMartin-NOAA pushed a commit to NOAA-EMC/GDASApp that referenced this pull request May 28, 2024
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants