MetOffice / opsinputs

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

Map channels to Var Channels #181

Closed orlewis closed 11 months ago

orlewis commented 1 year ago

This PR adds the functionality to map channels defined in JOPA to channels in Var for the varobs writer.

As part of this change the offset channels option has been removed as this can nw be covered using the varChannels mapping option. use_actual_channels has been replaced by a compress_var_channels. This determines whether to compress channel numbers together i.e 3, 5, 100 in just three indices or if do not compress this would have an array of size 100.

This will require changes to radiance yamls in Sith.

Requires this change to be coordinated. https://github.com/MetOffice/sith/pull/262

orlewis commented 1 year ago

I think there is still a need to put in some failure captures, incompatible options etc. But thought should have some more eyes take a look at it first.

DJDavies2 commented 1 year ago

This isn't compiling for me:

/home/h01/david.davies/cylc-run/opsinputs-181/share/mo-bundle/opsinputs/src/opsinputs/opsinputs_fill_mod.F90:1070:4:

1070 | if present(compressVarChannels) then | 1 Error: Missing '(' in IF-expression at (1)

orlewis commented 1 year ago

@ctgh and @james-cotton, This is the PR for adding the functionality needed for the geostationary radiances. The ctests have been passing but when I ran with sith kgo testing I got varobs failures with Satwind and Sonde http://fcm1/cylc-review/view/olewis?&suite=sith_map_chans_kgo&no_fuzzy_time=0&path=log/job/20210701T1200Z/glu_jopa_compare_output/07/job.out The change is mainly focused on how channels are manipulated and I had though that Satwind and Sonde are insulated from these particular changes but maybe I have missed something.

ctgh commented 1 year ago

@orlewis I believe James' recent optimisation PR in sith changed some of the KGOs due to using a different number of PEs. Also, this branch is out of date with develop. Finally I find that running rose suite-clean prior to running the KGO test can prevent some false positives from appearing.

orlewis commented 1 year ago

KGO is now passing (aprat from GPSRO which I think is failing the nightly tests aswell) http://fcm1/cylc-review/view/olewis?&suite=sith_kgo_map_chans231020&no_fuzzy_time=0&path=log/job/20210701T1200Z/glu_jopa_compare_output/03/job.out

orlewis commented 1 year ago

The latest KGO output is here http://fcm1/cylc-review/view/olewis?&suite=sith_map_chans_kgo_231023&no_fuzzy_time=0&path=log/job/20210701T1200Z/glu_jopa_compare_output/09/job.out all passing apart from GPSRO which is currently failing the main nightly KGO. Using the current nightly GPSRO varobs and cx output all comparisons match as seen in http://fcm1/cylc-review/view/olewis?&suite=sith_map_chans_kgo_231023&no_fuzzy_time=0&path=log/job/20210701T1200Z/glu_jopa_compare_output/11/job.out

orlewis commented 11 months ago

The routines opsinputs_fill_fillreal2d_norecords and opsinputs_varobswriter_fillchannumandnumchans now both contain long nested if blocks. What do you think about creating two helper functions to move some of that functionality outside of those functions? That might make the logic of the code a bit easier to follow and maintain. Just an idea - no problem if you think it won't be useful.

I think this would be useful but also at this point I think we should get this code in and ready to use for the Geostationary radiances so they can be finished for the global.

mikecooke77 commented 11 months ago

@orlewis there is some pressure to get the geos done before xmas. Is this ready to go. Has the latest code been re-run with the sith-kgo. If not could this be set to run asap?

orlewis commented 11 months ago

There is KGO from 31/10/23 but there has been one change since then which was adding IMDI. http://fcm1/cylc-review/taskjobs?per_page=15&user=olewis&no_fuzzy_time=0&suite=sith_map_chans_kgo_231031&task_status=runahead&task_status=waiting&task_status=held&task_status=queued&task_status=expired&task_status=ready&task_status=submit-failed&task_status=submit-retrying&task_status=submitted&task_status=retrying&task_status=running&task_status=failed&task_status=succeeded&page=2 there are failures related to GPSRO KGO but these were consistent with the nightly tests at the time.

Because it has been a month I''l rebuild and run them again today.

chawnharlow commented 11 months ago

Hi guys, I just need to let you know that David Simonin has requested that the GEOs get treated as priority which means that this PR should be seen as high priority. I've not been following the action here, but if you can wrap it up that would be greatly appreciated. Thanks.

ctgh commented 11 months ago

@chawnharlow We have two approvals so this can be merged if the KGO tests are passing. I can do the merge this afternoon if everything is OK.

orlewis commented 11 months ago

http://fcm1/cylc-review/view/olewis?&suite=sith_kgo_mapchans_231204&no_fuzzy_time=0&path=log/job/20210701T1200Z/glu_jopa_process_background_ssmis/01/job.out Getting this error across several obs on the JOPA run. Haven't been able to find where it is coming from this morning.

ctgh commented 11 months ago

varno number with the same dimension was recently added to ioda. Therefore I suspect you need to update all of the repositories in your local mo-bundle, rebuild, and rerun the KGO tests.

orlewis commented 11 months ago

KGO results are in http://fcm1/cylc-review/view/olewis?&suite=sith_kgo_mapchans_231204&no_fuzzy_time=0&path=log/job/20210701T1200Z/glu_jopa_compare_output/02/job.out

All passed.

ctgh commented 11 months ago

Good news. @mikecooke77 I have added the 'ready to merge' label if you would like to take one more look.