Propose adding "filter name" to each filter in each ob space yaml

Description

I am proposing to add an unused (by JEDI) key filter name to every filter in each ob space yaml and will match the comment that is already in place for each block. The reason for this is two fold:

When moving to JCB, all comments will be stripped from the yamls. Having this filter name, while potentially unused otherwise (JCB doesn't know it is unused), will keep some level of documentation about each filter and add ease of navigating through the yaml.
Another reason for this is that it potentially makes parsing and mass edits easier. (Hopefully with JCB this part will be easier or even unneeded with the observation chronicle--though the chronicle doesn't yet support anything for conventional data). The idea is that we can have any number of Bounds Check, for example, in each yaml but no way to really distinguish which Bounds Check when parsing or mass editing.

I've already tested such functionality with the following example in the ctest case and it ran without any issue:

         # Online regional domain check
         - filter: Bounds Check
           filter name: "Online regional domain check"
           filter variables:
           - name: airTemperature
           test variables:
           - name:  GeoVaLs/observable_domain_mask
           minvalue: 0.0
           maxvalue: 0.5

I wanted to get feedback and support for this change before I edit the 50+ files and every filter within those files.

@guoqing-noaa

@guoqing-noaa @SamuelDegelia-NOAA @SamuelDegelia-NOAA I wanted to get feedback and support for this change before I edit the 50+ files and every filter within those files. I think this could be really useful, but there could be things I'm overlooking.

Thanks

@delippi

@delippi This is a good idea!

I have some minor comments:

It may be better to use uname (unique name) instead of filter name. No matter what, each filter already has a "name", i.e. "filter: Bounds Check". Using uname may help reduce any confusions.
I suggest we use either camelCase or concatenation_using_underscore for the unique name, i.e. onlineRegionalDomainCheck or online_regional_domain_check. We may update JEDI codes in the future to use this extra information, for example, we can include them in the jdiag files. camelCase or concatenation_using_underscore is more friendly for downstream processing.
I suggest we make unique names for each filter, not just copy comments (although in lots of situations comments are already good options for unique names).
Thanks!

@guoqing-noaa

@guoqing-noaa I like those ideas. I think I slightly prefer snake_case over camelCase. I guess all we need to do then is give each an agreed uname. I most certainly will am missing something but here are some to start with

This is structured like
Comment: proposed uname

Accept, reject, or passivate (monitor): iuse
Reject all obs not in type: obs_type_filter
Reject all obs with QualityMarker > 3: quality_marker_filter
Time window filter: twindow
Duplicate Check: duplicate_check
Online regional domain check: online_regional_domain_check
Initial error assignment: initial_error_assignment
Error inflation based on pressure check (setupt.f90): pressure_check_error_inflation
Error inflation based on errormod (qcmod.f90): errormod_error_inflation
Error inflation when QualityMarker == 3 (read_prepbufr.f90): quality_marker_error_inflation
Error inflation when observation pressure < 100 hPa (read_prepbufr.f90): obs_press_error_inflation
Bounds Check (ObsError) : sanity_check_obs_error
Bounds Check (ObsValue)  sanity_check_obs_value
Create temporary ObsErrorData: create_temporary_ObsErrorData
Set ObsError set "error parameter" if < "max value": ermin
Set ObsError set "error parameter" if > "min value": ermax
Gross Error Check: gross
Gross Error Check: cgross*0.7 if QualityMarker=3: gross_0.7
Re-assign err ObsErrorData <--- TempObsErrorData after gross error check.: reset_errors

Some of those are pretty bad. This is a starting point. I used convinfo names where applicable (iuse, twindow, gross, ermax, ermin)

What about something like udescription instead of uname? Both options could work, but I slightly prefer udescription since uname or filter name sound more like concise titles. Using something like udescription leaves room for a bit more detail, which can help make these variables clearer to users and serve as lightweight documentation when comments are stripped.

@SamuelDegelia-NOAA

@SamuelDegelia-NOAA I'm fine with using udescription. I should check it also works and that filter name wasn't coincidentally a real key ha!

@delippi

@guoqing-noaa I like those ideas. I think I slightly prefer snake_case over camelCase. I guess all we need to do then is give each an agreed uname. I most certainly will am missing something but here are some to start with

This is structured like Comment: proposed uname

Accept, reject, or passivate (monitor): iuse
Reject all obs not in type: obs_type_filter
Reject all obs with QualityMarker > 3: quality_marker_filter
Time window filter: twindow
Duplicate Check: duplicate_check
Online regional domain check: online_regional_domain_check
Initial error assignment: initial_error_assignment
Error inflation based on pressure check (setupt.f90): pressure_check_error_inflation
Error inflation based on errormod (qcmod.f90): errormod_error_inflation
Error inflation when QualityMarker == 3 (read_prepbufr.f90): quality_marker_error_inflation
Error inflation when observation pressure < 100 hPa (read_prepbufr.f90): obs_press_error_inflation
Bounds Check (ObsError) : sanity_check_obs_error
Bounds Check (ObsValue)  sanity_check_obs_value
Create temporary ObsErrorData: create_temporary_ObsErrorData
Set ObsError set "error parameter" if < "max value": ermin
Set ObsError set "error parameter" if > "min value": ermax
Gross Error Check: gross
Gross Error Check: cgross*0.7 if QualityMarker=3: gross_0.7
Re-assign err ObsErrorData <--- TempObsErrorData after gross error check.: reset_errors

Some of those are pretty bad. This is a starting point. I used convinfo names where applicable (iuse, twindow, gross, ermax, ermin)

@delippi These look great! Thanks!

I suggest adding "check" to "iuse, twindow..., etc",
i.e. iuse_check, twindow_check, gross_check, ermax_check, ermin_check, etc.

sanity_check_obs_error and sanity_check_obs_value may be changed to bounds_check_obs_error, bounds_check_obs_value?

Is gross_0.7 gross_deflation_0.7?

thanks for the suggestions! I will go ahead and start adding these and we can finalize them at the PR stage. They should be really easy to mass edit at that point.

gross_0.7 or gross_deflation_0.7 both work IMO. Or is the question about what that filter actually does? It is for the gross check for QM==3 and the gross parameter gets relaxed to 70%.

Relevant code in GSI setupt

 ! modify gross check limit for quality mark=3
     if(data(iqc,i) == three ) then
        qcgross=r0_7*cgross(ikx)
     else
        qcgross=cgross(ikx)
     endif

@SamuelDegelia-NOAA

@SamuelDegelia-NOAA Would udescription be still some kind of similar to ucomment? :)

In the future, we would like to check the impact of each filter, like how many observations are affected by a filter, then we may use this unique name to help with this purpose. Currently, @HaidaoLin-NOAA has been creating diagnostic flags for the filters he wants to check the filtering impact, which is not that convenient. I personally prefer uname based on this purpose, i.e. they serve to identify a unique filter, not provide comments/descriptions. Per Donnie's proposal, those names are not that long to exclude them from being called names.

thanks for the suggestions! I will go ahead and start adding these and we can finalize them at the PR stage. They should be really easy to mass edit at that point.

gross_0.7 or gross_deflation_0.7 both work IMO. Or is the question about what that filter actually does? It is for the gross check for QM==3 and the gross parameter gets relaxed to 70%.

Relevant code in GSI setupt
 ! modify gross check limit for quality mark=3
     if(data(iqc,i) == three ) then
        qcgross=r0_7*cgross(ikx)
     else
        qcgross=cgross(ikx)
     endif

Thanks for the details. I think adding a little bit details without making it lengthy will be helpful, any of gross_deflate_0.7, gross_inflate_0.7, gross_multi_0.7 works for me. Thanks!

@guoqing-noaa

@guoqing-noaa Yes ucomment and udescription are fairly similar and I have no issue with using either of those. My thought was that the goal was to move the detailed comments into this variable name. But now that I see @delippi's proposed names, I see that they are much shorter and thus uname is probably fine.

@SamuelDegelia-NOAA

@guoqing-noaa Yes ucomment and udescription are fairly similar and I have no issue with using either of those. My thought was that the goal was to move the detailed comments into this variable name. But now that I see @delippi's proposed names, I see that they are much shorter and thus uname is probably fine.

Thanks, @SamuelDegelia-NOAA

Another benefit using uname is that we can use it to refer a particular filter in discussions, collaborations or even in publications. Thanks!

Navigation Menu

Repository navigation

Propose adding "filter name" to each filter in each ob space yaml #447

Description

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Propose adding "filter name" to each filter in each ob space yaml #447

Description

Description

Activity

delippi commented on Sep 19, 2025

guoqing-noaa commented on Sep 19, 2025

delippi commented on Sep 19, 2025

SamuelDegelia-NOAA commented on Sep 19, 2025

delippi commented on Sep 19, 2025

guoqing-noaa commented on Sep 19, 2025

delippi commented on Sep 19, 2025

guoqing-noaa commented on Sep 19, 2025

guoqing-noaa commented on Sep 19, 2025

SamuelDegelia-NOAA commented on Sep 19, 2025

guoqing-noaa commented on Sep 19, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions