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/bweir/#79 codas #119

Merged
merged 17 commits into from
Feb 15, 2023
Merged

Feature/bweir/#79 codas #119

merged 17 commits into from
Feb 15, 2023

Conversation

briardew
Copy link
Contributor

This should be everything needed for the GEOSana_GridComp for full CoDAS functionality (M2CC, SCREAM products, etc.).

tclune
tclune previously approved these changes Jan 1, 2023
Copy link
Collaborator

@tclune tclune left a comment

Choose a reason for hiding this comment

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

cmake changes ok

@briardew briardew added the 0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) label Jan 6, 2023
@briardew briardew requested a review from rtodling January 10, 2023 19:52
Copy link
Contributor

@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.

My guess is that by you having moved CO2 (and CO, but I don’t care about it) around you caused the code not to take CO2 correctly, with the consequence that CRTM is likely not seeing CO2 as it should see. This is my GUESS, of what’s is happening. I am in the process of testing my hypothesis.

But you cannot implement something and “think” that all you did has nothing to do w/ the typical things and assume it reproduces the standard cases: you need to test!

Also, I am rather puzzled by the way you are using the GSI to tracer analysis. I would have imagined that you would not need any changes in standard anavinfo at all. On What I would have expected is for you to add a tracer-specific anavinfo that would exercise mainly the tracer components. That is,

That would have something like this in the state_vector table:

state_vector::
!var level itracer source funcof
prse 73 0 met_guess prse
ps 1 0 met_guess prse
co2 72 1 chem_guess co2
ch4 72 1 chem_guess ch4
co 72 1 chem_guess co
clo 72 1 chem_guess clo
ch3cl 72 1 chem_guess ch3cl
n2o 72 1 chem_guess n2o
hno3 72 1 chem_guess hno3
hcl 72 1 chem_guess hcl
h2o 72 1 chem_guess h2o
::

That is, no meteorological quantities; you would need ps and the 3d-pressures since you need them for the vertical coordinates.

I think we need talk …

@rtodling
Copy link
Contributor

Here are also some general comments:

  1. you need to have RC files that are specific to the trace gases leaving the usual files alone. This applies to anavinfo and to gsi.rc.tmpl

  2. we need to talk about adding a test case for trace gases; something that can be run and tested from time to time when changes are made so we can be sure we don't make the mistake of changing your results inadvertently.

  3. I believe you edits of GSI_GridComp have changed how GSI handles CO2; also I believe you have changed the units of CO2 w/o regard for other part of the code that might need CO2

  4. your settings should be such as to minimize references to the meteorology. I don't think there will be a scenario when an system will use a met + trace gases analysis ... unless I am misunderstanding how the chem part of R21C will run.

  5. I swapped GSI_GridComp in your version w/ the default and I get zero diff; sure this breaks your use of the code, but it proves my point that the error is your edits of GSI_GridComp; we should talk about them.

I have a number of other smaller concerns and comments, but the items above make up the most important stuff.

@rtodling
Copy link
Contributor

Brad, just to let you know that I made changes that make the code reproduce the original GSI - the one presently in develop.
Notice also, that I changed the follow:

  1. placed TGAS rc files in etc/TGAS
  2. recovered original rc files (gsi.rc.tmpl and anavinfo) as they are in develop
  3. other minor edits
    I am ready to bring this to develop.

Now what I am doing next - already have it in my sandbox is a setting that runs an O3-only analysis. This will serve as a template for how to run TGAS-only analysis ... we'll need to talk for the latter - I will put those changes in another PR, as I want for this one to go in first.

@mathomp4 mathomp4 self-requested a review February 15, 2023 19:35
@rtodling rtodling merged commit d118152 into develop Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) codas Constituent Data Assimilation (CoDAS) related work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CoDAS functionality
4 participants