MetOffice / opsinputs

JEDI library generating VarObs and Cx files
BSD 3-Clause "New" or "Revised" License
4 stars 0 forks source link

Write varobs and varcx for Scatwind chosen solution #182

Closed james-cotton closed 9 months ago

james-cotton commented 11 months ago

This PR adds the capability to write VarObs/Cx files for scatterometer winds when we only want to output a single wind solution. In this configuration we read in the 4 ambiguous wind solutions and then select a single, chosen wind solution.

This new configuration, here named ScatwindChosen, will be needed for the initial implementation of JADA. Although not needed for JADA itself, we need to produce VarObs and VarCx files so that the new configuration can be added to the SITH nightly tests, but also so that scientific impact of chosen wind assimilation versus ambiguous wind assimilation can be evaluated in JOPA-VAR.

We then have two options for scatterometer VarObs: • Scatwind: output 4 ambiguous wind solutions along with prior probabilities for VAR • ScatwindChosen: output a single, chosen wind solution for VAR/JADA

For the VarObs we want to output varfields 4 (VarField_u) and 5 (VarField_v), which are filled from BiasCorrObsValue/windEastwardAt10M and BiasCorrObsValue/windEastwardAt10M respectively. I have added new tests for these.

The VarCx files are the same in both configurations. I realised there was no existing Cx test for the original Scatwind configuration so I have added a new test for that. Since the VarCx are the same there seemed no need to add a further test for ScatwindChosen.

SITH output I have tested the varobs and cx writer filters in my SITH branch and verified the files produced look correct: http://fcm1/cylc-review/view/frjd?&suite=sith_scatwindchosen_obstype&no_fuzzy_time=0&path=log/job/20210701T1200Z/glu_jopa_process_background_scatwindchosen/09/job.out The output between the two configs can be compared by looking at the txt files in /data/users/frjd/jedi/scatwind_single_solution/test_varobs_varcx/

If we diff the CX files we see that the output is identical apart from the number of locations in each file (due to slight difference in QC).

Looking at the first observation in the VarObs data section, we see that in scatwindchosen we have selected the channel 1 solution (referred to as level 1 in varobs) and the contents are equivalent to scatwindfor these lines, with only the field numbers changed - see the highlighted lines below

image

mikecooke77 commented 11 months ago

The majority of code changed is code copied from OPS. The issue with this is that there will be a case for merging new OPS changes into this repo. This could result in the changes you made here being lost.

It would be good to get @adammaycock opinion on how we want to deal with these changes? Should the same changes be made to the OPS code?

james-cotton commented 11 months ago

The code looks good to me I just want to get @adammaycock opinion on how we should deal with these changes with regards to OPS.

Hi @adammaycock, is there any issue with code in opsinputs diverging from ops?

adammaycock commented 11 months ago

The code looks good to me I just want to get @adammaycock opinion on how we should deal with these changes with regards to OPS.

Hi @adammaycock, is there any issue with code in opsinputs diverging from ops?

I don't think there's an issue. This change will never be required for OPS, so no point in mirroring it there for the sake of it. The previous requirement (desire) to keep the codes in sync reduces over time.

james-cotton commented 10 months ago

Hi @mikecooke77 are you happy to approve this now, given Adam's comment above?

mikecooke77 commented 10 months ago

The code looks good to me I just want to get @adammaycock opinion on how we should deal with these changes with regards to OPS.

Hi @adammaycock, is there any issue with code in opsinputs diverging from ops?

I don't think there's an issue. This change will never be required for OPS, so no point in mirroring it there for the sake of it. The previous requirement (desire) to keep the codes in sync reduces over time.

The code looks good to me I just want to get @adammaycock opinion on how we should deal with these changes with regards to OPS.

Hi @adammaycock, is there any issue with code in opsinputs diverging from ops?

I don't think there's an issue. This change will never be required for OPS, so no point in mirroring it there for the sake of it. The previous requirement (desire) to keep the codes in sync reduces over time.

My concern is that there could be a time when we want to pull the latest changes to OPS back into this repository, for instance when a new satellite is added to OPS. The way this is scripted (https://github.com/MetOffice/opsinputs/blob/develop/deps/update_ops_sources.sh) will lead to the changes in this PR being removed and the code will probably fall over. This is something that we have dealt with in the past by updating OPS instead of making changes in this repository.

james-cotton commented 10 months ago

My concern is that there could be a time when we want to pull the latest changes to OPS back into this repository, for instance when a new satellite is added to OPS. The way this is scripted (https://github.com/MetOffice/opsinputs/blob/develop/deps/update_ops_sources.sh) will lead to the changes in this PR being removed and the code will probably fall over. This is something that we have dealt with in the past by updating OPS instead of making changes in this repository.

What do you think @adammaycock? If we use that script to pull in changes from OPS then my local changes here would be removed. It looks to me like we either a) Mirror this change (and others like it) in OPS b) Do not use the script to copy in OPS code in bulk, and instead use it to bring in updates from OPS selectively through e.g. using xxdiff to do a merge c) Use the "stubs" approach here: https://github.com/MetOffice/opsinputs/blob/develop/deps/README.md. We would still need an approach to pull in updates for the code moved to the stubs folder though

james-cotton commented 10 months ago

In this last commit a4c3ad0, I have moved all the directories in deps that have modified code to the stubs folder. I have then removed those directories from update_ops_sources.sh as per the guidance. As this has to be done on a directory basis, there are a lot of files moved.

james-cotton commented 10 months ago

Thanks Mike. I think this is now ok to be merged

james-cotton commented 9 months ago

@yaswant please can this be merged when you have time. Thanks