-
Notifications
You must be signed in to change notification settings - Fork 17
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
Adding atmospheric composition variables for JCSDA needs #44
Conversation
Well, they are the same in an ideal gas. I understand that the difference is almost always |
I am in favor of this. I would also love to see host-model-specific areas so that sections that are for interoperability would be separate from variables that are only used by one host model. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few questions and concerns.
standard_names.xml
Outdated
@@ -368,13 +368,15 @@ | |||
<type kind="kind_phys" units="m s-1">real</type> | |||
</standard_name> | |||
</section> | |||
<section name="constituents"> | |||
<section name="atmospheric_composition"> | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need blank lines in XML files?
standard_names.xml
Outdated
<standard_name name="number_of_chemical_species"> | ||
<type kind="kind_phys" units="count">integer</type> | ||
</standard_name> | ||
<standard_name name="number_of_tracers"> | ||
<type kind="kind_phys" units="count">integer</type> | ||
</standard_name> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why add a blank link here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will remove the blank lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, would the xml be a bit more human-readable if we put spaces to separate the sections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am okay with any plan as long as it is consistent.
<standard_name name="mass_fraction_of_dust001_in_air" | ||
long_name="Dust bin1 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_dust002_in_air" | ||
long_name="Dust bin2 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_dust003_in_air" | ||
long_name="Dust bin3 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_dust004_in_air" | ||
long_name="Dust bin4 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_dust005_in_air" | ||
long_name="Dust bin5 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_sea_salt001_in_air" | ||
long_name="Sea salt bin5 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_sea_salt002_in_air" | ||
long_name="Sea salt bin2 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_sea_salt003_in_air" | ||
long_name="Sea salt bin3 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_sea_salt004_in_air" | ||
long_name="Sea salt bin4 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_sea_salt005_in_air" | ||
long_name="Sea salt bin5 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which set of bins is this? NGAC? ESA?
There are models and studies using different bin sizes as well as number of bins (e.g., more coarse bins to better capture the mass distribution in the atmosphere).
To be unambiguous, I think these should specify model of model type.
Maybe add ngac
to these names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used with the GOCART aerosol scheme. But can be used in other aerosol schemes. To be honest, the names are already very long, is it really necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used with the GOCART aerosol scheme. But can be used in other aerosol schemes. To be honest, the names are already very long, is it really necessary?
To answer this question, you would have to know if anyone using that name would expect that the quantity is the same as in their model. People using a variable that ends up having a different physical definition than what they thought is one of the reasons the CCPP was invented in the first place.
standard_names.xml
Outdated
long_name="Sulfate mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_sea_nitratet001_in_air" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what "nitratet" is. Well, in Norwegian, it is the single definite form of "nitrat" (salt of nitric acid) but I'm guessing that is not what this is.
If it is a typo, please fix it here and in the two bins below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a typo. I will correct this. Unnskyld!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Takk, det går bra.
standard_names.xml
Outdated
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_sea_nitratet002_in_air" | ||
long_name="Nitrate bin2 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="mass_fraction_of_sea_nitratet003_in_air" | ||
long_name="Nitrate bin3 mass fraction"> | ||
<type kind="kind_phys" units="kg kg-1">real</type> | ||
</standard_name> | ||
<standard_name name="volume_extinction_in_air_due_to_aerosol_particles_lambda1" | ||
long_name="Aerosol extinction at wavelength1"> | ||
<type kind="kind_phys" units="m-1">real</type> | ||
</standard_name> | ||
<standard_name name="volume_extinction_in_air_due_to_aerosol_particles_lambda2" | ||
long_name="Aerosol extinction at wavelength2"> | ||
<type kind="kind_phys" units="m-1">real</type> | ||
</standard_name> | ||
<standard_name name="volume_extinction_in_air_due_to_aerosol_particles_lambda3" | ||
long_name="Aerosol extinction at wavelength3"> | ||
<type kind="kind_phys" units="m-1">real</type> | ||
</standard_name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, are these universal wavelenths? If not, please attach a model name to each standard name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's tied to a data assimilation methodology that GMAO is performing. It is not conditioned by one model in particular.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, I would like to have only one variable where the wavelength is another dimension, so the lambda part would be removed. I don't have control over this as it is dependent on GMAO folks working on this specific methodology to make changes in the JEDI code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, that extra dimension could be a coordinate so that the bands are defined by the coordinate variable but right now, those numbers could mean different things to different users. Can a case be made that no other system could ever have a use for these variables (a strong claim)?
Another idea (here and above with the GOCART bins) is that if we can create sections where model-specific standard names live, maybe we should allow this sort of name. Thoughts? @climbfuji? @grantfirl? @peverwhee?
standard_names.xml
Outdated
long_name="Aerosol extinction at wavelength3"> | ||
<type kind="kind_phys" units="m-1">real</type> | ||
</standard_name> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for this blank line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for my own readability of the xml code... I have corrected this now.
no, they are precisely the exact same quantities... |
The concern about the aerosol scheme is valid. But I don't think we should add the scheme info in the names, they are already very long. I would suggest we create subsections where we specify the aerosol scheme in that case: atmospheric_composition > aerosols > GOCART > list of names. I think it is done somewhere where GFS is specified in the section title but not every variable has gfs in it. Would this solution be acceptable? |
I guess we took different chemistry courses :) My understanding is that they are the same as long as the volume |
This is similar to what I just wrote above. However, currently, the GFS variables are not pulled out into a separate host-model section for UFS (and other models that use the same GFS physics structures?). If we (i.e. you :) could create this sort of sectioning, I would be happy. If the group agrees with this plan, then I would suggest that you segregate them now and implement the sectioning in a new PR (as you originally proposed). |
That's just GitHub taking a break. I see the changes now. |
harvard course and book by Pr. D. Jacob https://acmg.seas.harvard.edu/education/introduction-atmospheric-chemistry chapter 1 section 1.1 first line... those are two different terms to define the exact same thing. regardless of volume and the number of molecules. |
Well, sure, but if you read the whole paragraph, you find:
So, yes, they are roughly equivalent in the Earth's atmosphere (to 1% or so) but they are not the same physical quantity. Okay, that took a few minutes: From Chemistry: The Central Science |
In our context, atmospheric chemistry, is considered the same for the reason that we apparently both know. So the unit of the volume mixing ratio and the mole fraction is mol /mol and therefore is the same and this is redundant in our context. My point was to mention that those variables are redundant and that mole fraction and volume mixing ratio from the same model and the same gas are very very likely to be bit identical... I think we should probably move on now... |
That's the thing, what is "our context"? For instance, our atmosphere model is also used to study different planets with quite different conditions so we try to not build unnecessary assumptions into the model. I would rather not build unnecessary assumptions into the CCPP if they could hinder future work. |
ha! i didn't know that this was considering extraterrestrial atmospheres here... as long as you have mixing ratios in mol/mol, which is used across the entire community this will be the same as a mole fraction. we can go on like this and bore to death everyone else on this PR... |
I have removed myself as a reviewer as these are names which are outside my area of expertise. I have requested @mattldawson as an alternate reviewer as this will overlap with the models that he works on. |
@jeromebarre @gold2718 What is the current status of this PR? Have you settled your discussion and agreed on a path forward? |
Not really sure. I've corrected what was possible to correct in the XML file. For the long discussion with @gold2718 I was just pointing out that there might be redundant concentration definitions. This is valid only in earth's atmosphere but not in outer space... If this is used to run models of very high-pressure atmospheres (e.g. venus) the redundant quantities are not. Maybe @gold2718 can be more specific on what he now needs to see, but I am not sure what is still expected on this PR to be merged. |
I guess it depends on whether we can reach agreement on future work on two fronts:
Of course, in trying to answer your question, I found another problem ( |
@jeromebarre We discussed the issues brought up in this PR at today's CCPP Framework meeting, and we proposed the following path forward:
The model-dependent variables (from what I can see) are
|
For 1. it is not consistent with the convention for other gases... Shouldn't this convention be about clarity and consistency? At least adopt consistency for the same unit quantities (here volume mixing ratio which is the same unit as mole mixing ratio in earth's atmosphere btw: mol/mol). I strongly recommend that the CF conventions adopt molecule condensed formulae such as For 2. I will make the changes. Could we have a subsection under atmospheric composition? Is it technically possible with the current xml file? |
@jeromebarre If you really want the CF conventions to change, then you need to reach out to them ... good luck. CCPP standard names need to follow those conventions unless we have a strong and good reason not to do so. Is this reason strong/good enough? A compromise that I could imagine would be to say that the rule/preference is to use the common name of the molecule, but that aliases using the chemical formula (is there an ascii-standard notation for it?) are allowed. Such an automatic translation/match of names/alies could even be baked into the CCPP framework. Just an idea, needs further discussion. |
I would ideally try to follow the Hill system that is the most widely used in databases: Also: https://en.wikipedia.org/wiki/Glossary_of_chemical_formulae |
@jeromebarre With regards to your second point it looks like having sub-sections currently won't work. However, I personally think that is a great idea. If you want to try and tackle it yourself then feel free, but otherwise we can just make an issue and have one of us work on it in a separate PR, as that way this particular PR won't be held up. Let me know if you have a preference. Thanks! |
Yes, I think we'd need to modify the Python script (I now remember this when reading the description I put for this PR). I can create sections that are placeholders for future subsections such as Composition - Trace Gas, Composition - Aerosols - GOCART, etc... |
@jeromebarre Just letting you know that I have created an issue (#48) for adding a subsection element. Once we get that issue resolved we can hopefully move your sections into their respective subsections, assuming the names are still necessary. |
@jeromebarre just checking in to make sure you aren't waiting for action on our end. From what I can tell the path forward is:
|
Yes sorry, I was on leave the last week and I have been busy with many other tasks lately. I will address this as soon as possible. Thanks for your patience. Note that, reverting the instances of o3 to ozone won't work for us at JCSDA right now. This will require us to change things in several areas of the JEDI code if we want to use that convention. So we will create another inconsistency somewhere else and also with other trace gases' naming. Wouldn't we want to resolve the trace gas convention naming before merging this? |
I've now added a section for GOCART aerosols. This group should be aware that there would be only scheme-specific variables for aerosol in 3D Earth system modelling, unfortunately. GOCART is one of the schemes with a low number of variables comparatively. It is also the most widely used in the US in operations because of this. This scheme can be ported in various global and regional models so in a sense it is model-independent. Another scheme that is also used in our DA context at JCSDA is CMAQ which includes many more variables. So CCPP needs to prepare for this one way or another. Happy to help with the discussion for this. Also looking forward to the meeting to resolve the ozone/o3 problem. Thanks for your patience. |
Following a productive discussion today, we have agreed that this PR can go through as-is (with a few cleanup changes as proposed by @jeromebarre), and a future PR (see Issue #49) will do two things for cleanup, consistency, and looking forward to future names:
We can discuss any changes to the above two points once I have opened that issue. |
I've just updated the description of the volume mixing ratio to long names. I've left the mole fraction items as is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this matches what we discussed yesterday, thanks for making those changes.
<type kind="kind_phys" units="mol mol-1">real</type> | ||
</standard_name> | ||
<standard_name name="volume_mixing_ratio_of_cfc113" | ||
long_name="CFC113 volume mixing ratio"> | ||
long_name="1,1,2-Trichloro-1,2,2-trifluoroethane volume mixing ratio"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow.
It looks like I don't have permission to merge while there are still requested changes. Can we get an approval from @gold2718? I am not sure if he is still actively working with the capgen effort. |
And then I hit the wrong button... sigh - I've reopened! |
Sorry, I forgot I had not revisited this. One thing that really helps is to hit the little circular arrows icon in the reviewers box. This pings that reviewer to ask them to re-review their change requests. |
Thanks @gold2718, I hadn't used that functionality before so I'll keep that in mind for the future. Merging now. |
Adding atmospheric composition variables for JCSDA needs.
I took the freedom to change constituent to atmospheric composition as this is a more generic term and is now more widely used in the scientific community,
I reordered this section as follows:
It would have been better to put them in XML subsections but the python script doesn't allow this. I could however update the python script in a different PR if we think the subsections are really needed.
Note that there redundancies in certain quantities such as for O3 and CO2 as volume mixing ratio and mole fraction is the same. I understand that some models especially on the meteorology side have O3 and CO2 concentrations as mole fractions but the chemistry modules as volume mixing ratios. Both should be kept to satisfy the needs but we should try to limit the redundancies as much as possible.