add a standard set of ADJ variables #247
Conversation
|
Thanks @mjlosch! This looks good to me. As an aside, adjoint variables can also be printed through the diagnostics package of the MITgcm. This means that they get written to That's just some rambling though, this is still very useful because I'm not sure anyone knows about or uses the adjoint+diagnostics capabilities. PS I'm hoping to get to your PR reviews and my open PRs over there next week |
|
Thanks @timothyas, I saw that something was already done for the 5 basic state variables, but it was not clear to me that this is universally possible. That's of course much better than what I have attempted here. It would be very good to have these "standard" diagnostics included in the diagnostics scheme (currently I am filling up my disks with all this unwanted output). If you already have started doing this somewhere, I'd be happy to help shape this into a contribution to MITgcm, maybe just for the variables I have added here? |
|
That sounds great. I can start a draft PR next week when I get to the other MITgcm PRs that I need to tend to. I'd honestly like to overhaul ad_dummy_in_stepping while we're at it, because it's getting a little ridiculous, and the way it's written makes adding control variables more laborious than necessary. |
|
But, in relation to this PR: LGTM |
| units='dJ/(m2 s-1)')), | ||
| 'ADJkapgm': dict(dims=['k', 'j', 'i'], attrs=dict( | ||
| standard_name="ADJkapgm", | ||
| long_name='dJ/dkapgm: Sensitivity to meridional surface stress', |
There was a problem hiding this comment.
Should be e.g. sensitivity to GM Intensity
| units='dJ/(m2 s-1)')), | ||
| 'ADJkapredi': dict(dims=['k', 'j', 'i'], attrs=dict( | ||
| standard_name="ADJkapredi", | ||
| long_name='dJ/dkapredi: Sensitivity to meridional surface stress', |
There was a problem hiding this comment.
e.g. sensitivity to Redi coefficient
|
Great! Thanks a lot @mjlosch! I see no problem with adding as much metadata as we can. My only concern is test coverage. We don't have an example that includes these fields, so we won't really know if there is a problem with them. Let's think about how to solve that problem in the long run. In the meantime, LGTM! |
|
Expanding a bit...it would be great if there were a central source of truth for MITgcm variable metadata that both xmitgcm and MITgcm itself could pull from. Perhaps a namelist file in the MITgcm source code, or a standalone database in yaml/json? |
|
Ug, sorry, it looks like I merged prematurely. I saw @timothyas "LGTM" and just clicked merge. I picked this up in #249. Any more typos we should fix over there? |
|
Sorry, that was my bad! I had just skimmed it but noticed the typos while working on the MITgcm PR mentioned above. I added some fixes over there. +1 for a variable metadata truth file :) |
for my ecco-like adjoint simulations, I would like to be able to read the hard coded standard adjoint output (per adjDumpFreq) with xmitgcm. Appending the dict
packages_state_variableslet's me do so.Also there's s small fix to one variable name and unit.