Skip to content
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

Feature/msienkie/acft bc cleanup tails #139

Merged
merged 6 commits into from
Dec 13, 2021

Conversation

gmao-msienkie
Copy link
Contributor

Modifications made to 'GEOSdas.csm' and 'fvssi' scripts to add the 'cleanup_tail=.true.' flag to the options for the two aircraft varBC methods.
The cleanup_tail process removes any entries from the aircraft temperature bias correction file that have not been updated for more than a year. This helps limit the size of the aircraft coefficient file.

When a particular tail number has no reports the bias correction coefficients and their background error are kept the same until that particular tail number resumes reports again - the background error is not inflated. Thus, this change will not be zero diff in the case where an aircraft reappears. If a tail has been missing for more than a year it is probably beneficial to discard the bias correction since the characteristics of the aircraft equipment may have changed or the equipment may have been replaced during the downtime.

string for VarBC type aircraft temperature bias correction.  The GSI
will delete coefficients for tails that have not been updated for
more than one year.
Added to the second instance of this substitution code in GEOSdas.csm
@gmao-msienkie gmao-msienkie requested a review from a team as a code owner December 6, 2021 21:02
@gmao-msienkie gmao-msienkie added the Non 0-diff The changes in this pull request are non-zero-diff label Dec 6, 2021
@gmao-msienkie
Copy link
Contributor Author

The cleanup_tail action is now optional, controlled via an environment variable.
setenv CLEANUP_TAIL 1 -> cleanup_tail=.true.
if the environment variable is not set, the default is not to set cleanup_tail, and the GSI by default sets that as false.

@rtodling
Copy link
Collaborator

rtodling commented Dec 13, 2021

Meta,

Do we really need to make this optional? That is to have an env variable controlling this? Perhaps you want it that way to get it to be backward compatible ...

Copy link
Collaborator

@rtodling rtodling left a comment

Choose a reason for hiding this comment

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

... I think I understand what is controlled by this env var; but should we make the default env var 1 instead of 0 ... ideally we don't have to tell OPS to add another env var. And 0 would only be needed in cases we are trying the reproduce a previous run, no?

@gmao-msienkie
Copy link
Contributor Author

I remember you had said something about increasing the array size would be better than cleaning up the tails. I wanted to add an option to allow people to clean tails without manually editing the gsi.rc. I agree when you are trying to reproduce a prior run you wouldn't want to be changing the ceofficients in the cftbias file in case some tail came back after being missing for a year. I am in favor with taking out the year-old 'stale' coefficients so if you feel that the default should be '1' (remove old coefficients) that is fine with me.

@rtodling
Copy link
Collaborator

Ok - I will merge as is, and will change the default when I make fold in other (unrelated) changes of my own.

Copy link
Collaborator

@rtodling rtodling left a comment

Choose a reason for hiding this comment

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

Actually, I am looking again and I am realizing that the way you changed this, when the env var is set to zero the gsi.rc after being edited will have a line:

">>>AIRCFT_BIAS<<<"

like this in the namelist and the fortran code is going to choke! We need to define the if differently ...

@gmao-msienkie
Copy link
Contributor Author

I tested this in a run. When I had the enviroment variable set to 1 the CLEANUP_TAIL was set to TRUE. When I commented out the definition, the first time around it had CLEANUP_TAIL as TRUE because I think the job inherited the environment variable or something. When I independently submitted a segment it set the undefined CLEANUP_TAIL as FALSE. I am trying to run a segment where I have the environment variable set to 0 now so I can check what it does.

My code looks like this - it continues to set the >>>AIRCFT_BIAS<<< as needed and then appends the value of cleanup_tail to the substitution string if the environment variable is set to true (1).

 case 2:
      set cftstring = "aircraft_t_bc=.true.,"
      if ( $CLEANUP_TAIL ) set cftstring = "$cftstring cleanup_tail=.true.,"
      echo "s/>>>AIRCFT_BIAS<<</$cftstring/g"  >> sed_file
      echo 'Setting aircraft_t_bc to true, using VV.VV^2 bias correction'
      breaksw

@rtodling
Copy link
Collaborator

Sorry - I missed it ... all fine!

Copy link
Collaborator

@rtodling rtodling left a comment

Choose a reason for hiding this comment

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

ok - sorry - all good

@rtodling rtodling merged commit 361c4cd into develop Dec 13, 2021
@gmao-msienkie gmao-msienkie deleted the feature/msienkie/acftBC_cleanup_tails branch March 9, 2022 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Non 0-diff The changes in this pull request are non-zero-diff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants