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

File output lacks OMA diagnostics after JEDI minimization #1273

Closed
ypwang19 opened this issue Sep 5, 2024 · 11 comments · Fixed by #1274
Closed

File output lacks OMA diagnostics after JEDI minimization #1273

ypwang19 opened this issue Sep 5, 2024 · 11 comments · Fixed by #1274
Assignees

Comments

@ypwang19
Copy link
Collaborator

ypwang19 commented Sep 5, 2024

After running minimization with JEDI, the OMA variable was not written out to diagnostic files. Adding prints frequency to the final section in the YAMLs in GDASApp parm can turn on the OMA diagnostics.

This issue is opened to track the changes needed to modify the YAMLs for minimization.

@RussTreadon-NOAA
Copy link
Contributor

Work for this issue will be done in feature/anlomb_diag.

The following changes have been committed to feature/anlomb_diag

  • 307a6d5 - add prints with frequency PT3H to 3dvar_fgat_gfs_aero.yaml.j2 and 3dvar_gfs_aero.yaml.j2 in parm/aero/variational/
  • b57d559 - add final_prints_frequency: PT3H to parm/atm/jcb-base.yaml.j2
  • 050add7 - add final_prints_frequency: PT3H to parm/snow/jcb-base.yaml.j2

@RussTreadon-NOAA
Copy link
Contributor

Tagging @jiaruidong2017 for awareness.

@RussTreadon-NOAA
Copy link
Contributor

@guillaumevernieres , we have

final:
  diagnostics:
    departures: oman
  increment:

in parm/soca/3dvarfgat.yaml.

Yaping and Cory found it necessary to add a prints section to get analysis minus observation diagnostics written to the diagnostic files. For example, 307a6d5 modified parm/aero/variational/3dvar_fgat_gfs_aero.yaml.j2 to

final:
  diagnostics:
    departures: anlmob
  prints:
    frequency: PT3H
  increment:

We may need to make a similar change in marine DA templates.

@RussTreadon-NOAA
Copy link
Contributor

Initially surprising finding. test_gdasapp_atm_jjob_var_run fails when run from g-w PR #2805 at f445e28 with changes from feature/anlomb_diag added to sorc/gdas.cd.

A check of the log file shows that the reference check fails

0: Run: Finishing oops::Variational<FV3JEDI, UFO and IODA observations> with status = 0
0: terminate called after throwing an instance of 'oops::TestReferenceMissingReferenceLineError'
0:   what():  TestReference: Missing reference file line corresponding to test output Line#:90
0: Test line: 'CostJb   : Nonlinear Jb = 0.0000033199535370'
srun: error: h21c46: task 0: Aborted (core dumped)
srun: Terminating StepId=66180083.0

With prints added to the yaml, we get the following extra printout

CostJb   : Nonlinear Jb = 0.0000033199535370
CostJo   : Nonlinear Jo(AMSUA N19) = 39476.8510817764617968, nobs = 73667, Jo/n = 0.5358824315063252, err = 6.2697697603632934
CostJo   : Nonlinear Jo(sondes) = 10639.2476170353256748, nobs = 4255, Jo/n = 2.5004107208073623, err = 10.9811231401273197
CostFunction: Nonlinear J = 50116.0987021317414474

This printout is not in test/atm/global-workflow/3dvar.ref. Hence, the check throws an error and the test fails.

We need to update the atmosphere 3dvar.ref as part of GDASApp PR #1274.

@RussTreadon-NOAA
Copy link
Contributor

@guillaumevernieres , not sure if adding

  prints:
    frequency: PT3H

is something we want for marine DA but b69af09 adds this to the finals section of

  • parm/soca/marine-jcb-base.yaml
  • parm/soca/variational/3dvarfgat.yaml

We can revert this commit if it is not necessary / desired.

@CoryMartin-NOAA
Copy link
Contributor

@RussTreadon-NOAA G doesn't have this issue because he has output: writing out the analysis states. Since we didn't do that from JEDI, we needed to add prints after a PR earlier this summer changed when the final diagnostics are computed.

@RussTreadon-NOAA
Copy link
Contributor

@CoryMartin-NOAA , does this mean the parm/soca changes committed at b69af09 are not necessary?

@CoryMartin-NOAA
Copy link
Contributor

@RussTreadon-NOAA I don't think they're necessary but I also don't think they will harm anything

@RussTreadon-NOAA
Copy link
Contributor

@CoryMartin-NOAA , OK, I'll leave b69af09 alone.

@RussTreadon-NOAA
Copy link
Contributor

@ypwang19 , do you have any more commits or changes to make to PR #1274? If not, it's ready for review.

@ypwang19
Copy link
Collaborator Author

ypwang19 commented Sep 6, 2024

@ypwang19 , do you have any more commits or changes to make to PR #1274? If not, it's ready for review.

It looks good to me, thank you!

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 a pull request may close this issue.

3 participants