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

Feature/levels metadata #28

Merged
merged 18 commits into from
Nov 29, 2023
Merged

Feature/levels metadata #28

merged 18 commits into from
Nov 29, 2023

Conversation

phlndrwd
Copy link
Collaborator

@phlndrwd phlndrwd commented Nov 17, 2023

Addition of vertical meta/data to NetCDF files for outputs that adopt JEDI variable conventions.

This PR will be part of a coordinated merge with:

Test outputs (LFRic-Lite, LFRic-JEDI, & MONIO): http://fcm1/cylc-review/taskjobs/punderwo/?suite=levels_metadata_01

…fields appropriately. Changed position if LFRic and JEDI in eVariableConventions enum so that LFRic is at position 1 and can be used instead of bool, if necessary.
@mo-joshuacolclough
Copy link
Contributor

Failing on develop with LFRic-JEDI:

Simplest test that is failing is the ctest_lfricjedi_xios_monio_comparison test. Currently looking into it...

Full output: http://fcm1/cylc-review/view/jcolclou?&suite=check_monio_change__201123&no_fuzzy_time=0&path=log/job/1/ctest_lfricjedi_xios_monio_comparison_lfric__spice_gnu/02/job.out

Summary of failure:

10: 20231120131301.016+0000testCompareXIOSMONIOStateRead> Reading XIOS(0) done.
10: testCompareXIOSMONIOStateRead> xiosState: 
10: 
10: ---------- State print ----------
10: Valid time: 2018-04-14T21:00:00Z
10: Number of variables: 4
10: Locally owned data num: 30384
10: 
10: 'eastward_wind'
10:   Min=-5.16192068802978099e+01, Max=8.38294119533246800e+01, RMS=1.64436981523366548e+01
10: 'northward_wind'
10:   Min=-5.18142921369839655e+01, Max=4.71832627877766768e+01, RMS=7.17428459853106837e+00
10: 'potential_temperature'
10:   Min=2.37215478585606320e+02, Max=5.73111180066490306e+03, RMS=1.15127835739124657e+03
10: 'skin_temperature'
10:   Min=2.15627294832477958e+02, Max=3.12638033249410739e+02, RMS=2.87465032112199594e+02
10: ---------------------------------
10: 
10: testCompareXIOSMONIOStateRead> Reading MONIO(0)...
10: testCompareXIOSMONIOStateRead> Reading MONIO(0) done.
10: testCompareXIOSMONIOStateRead> monioState: 
10: 
10: ---------- State print ----------
10: Valid time: 2018-04-14T21:00:00Z
10: Number of variables: 4
10: Locally owned data num: 30384
10: 
10: 'eastward_wind'
10:   Min=-5.16192068802978099e+01, Max=8.38294119533246800e+01, RMS=1.64436981523366548e+01
10: 'northward_wind'
10:   Min=-5.18142921369839655e+01, Max=4.71832627877766768e+01, RMS=7.17428459853106837e+00
10: 'potential_temperature'
10:   Min=2.37215478585606320e+02, Max=4.80996841150037835e+03, RMS=9.46595593171428391e+02
10: 'skin_temperature'
10:   Min=2.15627294832477958e+02, Max=3.12638033249410739e+02, RMS=2.87465032112199594e+02
10: ---------------------------------
10: 
10: Test "lfricjedi/testCompareXIOSMONIOStateRead" failed with unhandled eckit::Exception: Assertion failed: testCompareXIOSMONIOStateRead> XIOS and MONIO reads do not match. in testCompareXIOSMONIOStateRead, line 124 of /home/h02/jcolclou/cylc-run/check_monio_change__201123/share/mo-bundle/lfric-jedi/test/bespoke/IO/XIOSMONIOComparison.h

It seems that the potential_temperature values being read in no longer match what XIOS is reading. That is the theta field which is a full_level field missing the surface.

@mo-joshuacolclough
Copy link
Contributor

Failing on develop with LFRic-JEDI:

Simplest test that is failing is the ctest_lfricjedi_xios_monio_comparison test. Currently looking into it...

Full output: http://fcm1/cylc-review/view/jcolclou?&suite=check_monio_change__201123&no_fuzzy_time=0&path=log/job/1/ctest_lfricjedi_xios_monio_comparison_lfric__spice_gnu/02/job.out

Summary of failure:

10: 20231120131301.016+0000testCompareXIOSMONIOStateRead> Reading XIOS(0) done.
10: testCompareXIOSMONIOStateRead> xiosState: 
10: 
10: ---------- State print ----------
10: Valid time: 2018-04-14T21:00:00Z
10: Number of variables: 4
10: Locally owned data num: 30384
10: 
10: 'eastward_wind'
10:   Min=-5.16192068802978099e+01, Max=8.38294119533246800e+01, RMS=1.64436981523366548e+01
10: 'northward_wind'
10:   Min=-5.18142921369839655e+01, Max=4.71832627877766768e+01, RMS=7.17428459853106837e+00
10: 'potential_temperature'
10:   Min=2.37215478585606320e+02, Max=5.73111180066490306e+03, RMS=1.15127835739124657e+03
10: 'skin_temperature'
10:   Min=2.15627294832477958e+02, Max=3.12638033249410739e+02, RMS=2.87465032112199594e+02
10: ---------------------------------
10: 
10: testCompareXIOSMONIOStateRead> Reading MONIO(0)...
10: testCompareXIOSMONIOStateRead> Reading MONIO(0) done.
10: testCompareXIOSMONIOStateRead> monioState: 
10: 
10: ---------- State print ----------
10: Valid time: 2018-04-14T21:00:00Z
10: Number of variables: 4
10: Locally owned data num: 30384
10: 
10: 'eastward_wind'
10:   Min=-5.16192068802978099e+01, Max=8.38294119533246800e+01, RMS=1.64436981523366548e+01
10: 'northward_wind'
10:   Min=-5.18142921369839655e+01, Max=4.71832627877766768e+01, RMS=7.17428459853106837e+00
10: 'potential_temperature'
10:   Min=2.37215478585606320e+02, Max=4.80996841150037835e+03, RMS=9.46595593171428391e+02
10: 'skin_temperature'
10:   Min=2.15627294832477958e+02, Max=3.12638033249410739e+02, RMS=2.87465032112199594e+02
10: ---------------------------------
10: 
10: Test "lfricjedi/testCompareXIOSMONIOStateRead" failed with unhandled eckit::Exception: Assertion failed: testCompareXIOSMONIOStateRead> XIOS and MONIO reads do not match. in testCompareXIOSMONIOStateRead, line 124 of /home/h02/jcolclou/cylc-run/check_monio_change__201123/share/mo-bundle/lfric-jedi/test/bespoke/IO/XIOSMONIOComparison.h

It seems that the potential_temperature values being read in no longer match what XIOS is reading. That is the theta field which is a full_level field missing the surface.

Requires a small change in LFRic-JEDI to handle the new metadata

@phlndrwd phlndrwd marked this pull request as ready for review November 22, 2023 18:12
Copy link
Contributor

@mo-joshuacolclough mo-joshuacolclough left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks looks good. Haven't re-tested, but we've discussed previous issues and my understanding is that those are now fixed. Cheers

consts::kNamingConventions[consts::eJediNaming];
std::string variableConvention =
isLfricConvention == true ? consts::kNamingConventions[consts::eLfricConvention] :
consts::kNamingConventions[consts::eJediConvention];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent slightly off compared to above line

@@ -373,7 +373,7 @@ void monio::Metadata::deleteVariable(const std::string& varName) {

void monio::Metadata::clear() {
oops::Log::debug() << "Metadata::clear()" << std::endl;
dimensions_.clear();
// dimensions_.clear(); // Dimensions are required for correct writing of subsequent variables.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to remove commented out code if it's not needed anymore

@phlndrwd phlndrwd merged commit 083e3c1 into develop Nov 29, 2023
4 checks passed
@phlndrwd phlndrwd deleted the feature/levels_metadata branch December 1, 2023 13:27
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.

2 participants