Feature/query cxx types ne aircraft altitude#1071
Merged
srherbener merged 275 commits intodevelopfrom Dec 7, 2022
Merged
Conversation
…e redundant yaml files from testinput.
* Add bufr to ioda conversion for IASI, ERAS/DB AMSU-A and MHS for ncar-bufr2nc-fortarn utility * Add tests and associated test input/output files for ncar-bufr2nc-fortarn utility * Add NCEP BUFR tables for normal-feed and RARS/DB satellite radiance data * Add YAML files, tests, and test reference for normal-feed and db/ears AMSU-A * Fix coding norm * Rename NCEP BUFR tables for satellite radiance data * Add yaml files, tests, and test input/output files for MHS (normal-feed and RARS/DB) t * Update Split.cpp * Update DataObject.cpp * Update IodaEncoder.cpp * Update bufr_ncep_1bamua.yaml * Update bufr_ncep_1bmhs.yaml * Update bufr_ncep_esamua.yaml * Update bufr_ncep_esmhs.yaml * Fixed coding norms issue. * Fixed some little issues. Co-authored-by: Ronald McLaren <ronald.mclaren@noaa.gov>
Updated documentation for dimensions.
* Update snowadpsfc * Update bufr_snow_adpsfc.yaml * Update testoutput * Update yaml files and testinput and testoutput data * Reduce input data size to 20% and update output * Reduce input data size to 250KB and update testoutput
…responding test output files (#764)
…that deals with group by fields.
* Added adpupa prepbufr yamls w/o group_by field with input adpupa prepbufr file * adpupa netCDF files w/o group_by field * Updates made after ioda-validate.x run. * Changed Quality Mark group from "ObsQualityMark" to "QualityMarker"
Contributor
Author
|
@srherbener I made a brand new build and the errors were the same. I also did git pulls and merges between develop and my branch and the same error popped up. Email me and we can try to link up today or later this week after the Hera work |
Collaborator
@nicholasesposito thanks for trying that experiment. Let me see if I can reproduce this on my Mac, and see what's wrong. Later this afternoon I have time so we can meet then if necessary. |
Collaborator
|
I'm getting a match on my Mac with what CI testing is reporting. Only one test fails: and you are saying that you are seeing the error you reported earlier: Correct? If so then it looks like to me that your develop branch in ioda (not ioda-converters) is out of sync with github. Note your feature branch in ioda-converters appears to be in sync with develop (as you mentioned). In your build area, run If you are getting the error I see on my Mac, then I think it's a matter of validating the new results in the output file and then updating the test reference file. Thanks! |
Contributor
Author
|
@srherbener So for the first ctest you mentioned, it's so weird to me because that one passes. I get the warnings (which are expected), but I don't get the "DIFFER" messages. |
Collaborator
|
@nicholasesposito that is strange. I don't have access to Hera, but if we do a google meet while you drive perhaps we can figure out what is wrong. I'll send a calendar invite. |
Contributor
Author
|
@srherbener @CoryMartin-NOAA Some changes were made to the code thanks to @rmclaren. I'm going to see what happens when I update the code on Hera when it comes back online. If we get the same error on Hera as the other machines, then we have success and I'll update the output and push the new changes. I'll keep you all updated. |
Contributor
Author
|
Good news. The changes that @rmclaren made ended up being the fix! The changes caused the tests on Hera (the odd-computer out) to fail, and those failures were the same failures that were found on the other machines. I updated the testoutput, re-built, and re-ran the ctests on both machines (Hera and Orion), and how all of them work. @srherbener please check your Mac's runs and any others. Please let me know what you find if you're not getting what I'm getting. And afterward, if everything is to your liking, please approve this PR. Thank you to everyone @srherbener for being patient and @CoryMartin-NOAA and @rmclaren for all of your generous insights and help. |
Collaborator
|
@nicholasesposito great news! I get identical results on my Mac and on Orion to those from the ctests (ie all ctests pass except for the bufr coding norms test). Almost there! |
srherbener
reviewed
Dec 7, 2022
Collaborator
srherbener
left a comment
srherbener
left a comment
There was a problem hiding this comment.
Looks good! Thanks for your patience with getting the tests straightened out. I have just one very minor comment.
Contributor
Author
|
Changes made and |
srherbener
approved these changes
Dec 7, 2022
This was referenced Dec 8, 2022
Contributor
Author
|
Project has been closed. Thanks everyone for your help and your patience. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
In aircraft data, there are a number of height variables that, in the prepqc code, are used to determine the aircraft flight level. Multiple mnemonics exist in a number of the AIRCFT datatypes. We need to add an enhancement to the feature/query_cxx code that will go through all of the data and variables to determine which variable we will want to use. There is logic within the prepqc code to help with this.
In prepbufr processing, it finds the flight level from the list of flight-level related mnemonics in the following order: PRLC, IALT, PSAL, FLVL, HEIT, HMSL, and FLVLST
Much like the Datetime variable in the ioda-converter yamls, this code will be placed in the src/bufr/BufrParser/Exports/Variables/ directory.
For some reason, previous aircraft yamls, inputs and outputs were removed. I re-added them.
This PR also ensures all of the outputs datatypes are correct.
Please build with ioda branch: feature/NE_reduce_aircft_vars_ObsSpace to get zero warnings
And for Ron, the file you used as an input for your new subsets test was different than the one I had. I re-ran using the input that I am using and updated the output file.
The main changes (which I will update in the first comment) are in the files:
iodaconv/CMakeLists.txt
iodaconv/src/bufr/BufrParser/Exports/Export.cpp
iodaconv/test/testinput/gdas.t12z.aircft.tm00.bufr_d
iodaconv/test/testinput/gdas.t12z.aircar.tm00.bufr_d
iodaconv/test/testinput/gdas.t12z.acft_profiles.prepbufr
iodaconv/test/testinput/bufr_ncep_aircft_noAMDAR103.yaml
iodaconv/test/testinput/bufr_ncep_aircft_AMDAR103.yaml
iodaconv/test/testinput/bufr_ncep_aircar.yaml
iodaconv/test/testinput/bufr_ncep_prepbufr_aircft.yaml
iodaconv/test/testoutput/gdas.t12z.aircft_noAMDAR103.tm00.nc
iodaconv/test/testoutput/gdas.t12z.aircft_AMDAR103.tm00.nc
iodaconv/test/testoutput/gdas.t12z.aircar.tm00.nc
iodaconv/test/testoutput/gdas.t12z.acft_profiles.prepbufr.nc
iodaconv/test/testoutput/bufr_specifying_subsets.nc
and the new files:
iodaconv/src/bufr/BufrParser/Exports/Variables/AircraftAltitudeVariable.cpp
iodaconv/src/bufr/BufrParser/Exports/Variables/AircraftAltitudeVariable.h
3 older files are removed:
rm 'test/testinput/acft_profiles.data_thinned'
rm 'test/testinput/aircftpro_prepbufr.yaml'
rm 'test/testoutput/aircraft_prepbufr_acftprofiles.nc'
Requirements
This will be a new feature. It will not require any new software dependencies. The new code will the amount of height variables needed in the aircraft bufr yamls. As of now, 6 are required, with an additional one (PRLC aka "pressure") being used for height calculations.
Please build with ioda branch: feature/NE_reduce_aircft_vars_ObsSpace to get zero warnings
Acceptance Criteria (Definition of Done)
This will be finished when
[ ] All the previous tests pass
[ ] Comparison with input bufr data confirms that the software works
[ ] A new test will be written to determine if future additions break the software.
[ ] The branch is merged to feature/query_cxx
Dependencies
#1044
#1059
Issue(s) addressed
fixes #997
fixes #974
Should be merged at the same time as
ioda/