-
Notifications
You must be signed in to change notification settings - Fork 2
Observation callsign changes for GPSRO #261
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
base: develop
Are you sure you want to change the base?
Conversation
|
Out of interest, how has this bug not caused a problem (difference from OPS). Does VAR do anything with the rising / setting flag? Also, I assume this change will result in kgo differences in nightly sith / malak testing, so we'll need to run those tests and generate new kgo before merging this PR. If that's the case, we'll set the |
|
Hi Adam, it hasn't caused any actual issues anywhere. Before, opsinputs cut off the very last digit of the callsign so the rising and setting information was lost at this point and is needed for current RO work. For context, please see the graphs in ssa-desroziers-2025. |
ctgh
left a comment
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.
Thanks for this fix. It looks like you need to add the new variable to the test netCDF file.
|
One of the ctests is failing for me: test_opsinputs_varobswriter_071_VarField_bendingangle There are also some coding norms failures. |
…hable else statement. Also tidied up to conform to coding norms.
|
I have just made a change to the initial commit to remove the else statement and include the variable in the constructor. I have also removed any white space from the file. I have checked the varobs file to see if the change is implemented and it has, with the last digit of the callsign only showing 1s and 0s. It is located here: |
I believe for the ctests you will need to recreate the file using the python script (https://github.com/MetOffice/opsinputs/blob/develop/test/generate_unittest_netcdfs.py) as part of this PR. |
… changes (adding variable for the observation callsign).
|
So, the change tells me that 140 files have been modified. Surely this is not right? |
|
I also thought that there were so many modified files. |
|
It appears that this changeset is the rogue one: 89ff34b |
|
So to make sure I have this right, just push the |
I would assume that the only data file to change should be |
|
This has happened before - it seems to occur whenever the underlying software stack is updated, which causes small differences in the netCDF files. Previously we have just updated all of the files even if they are unrelated to the PR. |
|
Not sure what to make of the CI runner. @matthewrmshin is this because we are using an older version of the JCSDA container? Should we move this to just using the azure UKMO CI as it will only ever be run in this environment? |
|
CI should now be using our own container running on our own self hosted runner. |
|
Build is now failing with a compiler error: /var/tmp/opsinputs/pr-261/opsinputs/src/opsinputs/GnssroStationIDMetOffice.cc:59:41: error: no matching function for call to 'missingValue(const int&)'
59 | const int missing = util::missingValue(missing);
| ~~~~~~~~~~~~~~~~~~^~~~~~~~~
In file included from /tmp/build-and-test-j5MEtK/ioda/include/ioda/ObsDataVector.h:24,
from /var/tmp/opsinputs/pr-261/opsinputs/src/opsinputs/GnssroStationIDMetOffice.cc:24: |
ctgh
left a comment
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.
Thanks, looks good!
|
There are failures in the ctest test_opsinputs_varobswriter_071_VarField_bendingangle. |
We found a bug in which opsinputs did not output the rising and setting flags for GPSRO into varobs.
We added a quality flag for the phase of occultation, rising (1) or setting (0), on the end of the observation callsign and split the last 7 characters of the callsign to include this flag at the end of the callsign.
An example is found in
/home/users/liam.hall/desktop/opsinputs_pr/varobs.txtstarting from line1418. The expected output is a 16 digit callsign with the last character to be either a 1 or a 0 - the phase flag.