Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve profile QC output #54

Merged
merged 2 commits into from
Jul 6, 2023
Merged

Improve profile QC output #54

merged 2 commits into from
Jul 6, 2023

Conversation

twsearle
Copy link
Collaborator

@twsearle twsearle commented Jul 5, 2023

Description

Quality control information was missing or wrong for whole-profile variables such as OBSERVATION_QC, POTM_QC, and PSAL_QC. I have also added information for DEPTH_QC, JULD_QC

This change fixes these variables as well as adding test output for writing a sub-set of profile observations.

There still remains missing QC information for the QC_FLAGS:

  • DEPTH_QC_FLAGS
  • OBSERVATION_QC_FLAGS
  • POSITION_QC_FLAGS
  • JULD_QC_FLAGS

As far as I know, these fields were not used by the old OPS or by NEMO/NEMOVAR. At least some of these fields have no meaning for JOPA, however it is possible we might want to fill out the depth quality control flags, as depth is considered an observed variable at the moment and the QCFlagsData does exist.

Tests

 * OBSERVATION_QC only rejects if all data rejected at a given location
 * fix whole profile QC variables for profiles
 * add test of profiles obs to write behaviour
@twsearle twsearle requested a review from s-good July 5, 2023 14:43
@s-good
Copy link
Collaborator

s-good commented Jul 5, 2023

Thanks for making the changes. I believe we do need DEPTH_QC to be filled out. This is used by the feedback file reader in NEMO. At the moment we are getting away with it not filled out because it has a negative fill value and the code checks for flag <= 2, but probably we shouldn't rely on this as other code might not work this way. Having said that, in the OPS feedback writer it looks like it is filled with -99999 where there is no valid depth or 1 if there is a valid depth, so it is actually quite similar to what we already have.

@twsearle
Copy link
Collaborator Author

twsearle commented Jul 5, 2023

Great, thanks @s-good. Would it be better to add on a DEPTH_QC change here or merge this promptly and make the change in a subsequent PR? How about JULD_QC and POSITION_QC? Are we best off just setting all of these to a default of 0?

@s-good
Copy link
Collaborator

s-good commented Jul 6, 2023

Great, thanks @s-good. Would it be better to add on a DEPTH_QC change here or merge this promptly and make the change in a subsequent PR? How about JULD_QC and POSITION_QC? Are we best off just setting all of these to a default of 0?

Just a note to say that we discussed this earlier and decided that these variables should be set according to the contents of new diagnostic flags (e.g. a PositionReject diagnostic flag). Even if these are not being set in the current processing yaml configuration, it gives us the option of using them in the future if we want to.

@twsearle
Copy link
Collaborator Author

twsearle commented Jul 6, 2023

set according to the contents of new diagnostic flags (e.g. a PositionReject diagnostic flag). Even if these are not being set in the current processing yaml configuration, it gives us the option of using them in the future if we want to.

this is in the latest commit now.

Copy link
Collaborator

@s-good s-good left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes. It all looks good to me.

@twsearle twsearle merged commit b78e395 into develop Jul 6, 2023
@twsearle twsearle deleted the feature/improve-qc-output branch July 6, 2023 17:03
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants