- 
                Notifications
    You must be signed in to change notification settings 
- Fork 340
[WIP]: Start bringing in carbon isotopes in streams #3561
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
base: b4b-dev
Are you sure you want to change the base?
Conversation
…e for CTSM, this compiles with intel on Derecho
… in the local object
…into carbon_isotopes_in_streams
… CTSM stream base type
…ve memory when appropriate
…e method a bit, so that varnames and name aren't sent in, also have to make sdat public, so that the dshr_fldbun_getFldPtr method can use it's internal components
…s for C13/C14 timeseries handling
…ilename was incorrect as well as a typo for C13 in the Advance and Interp routines
| The read is working now, so it needs more testing to ensure it's correct as well as review and some further updates we'll figure out in the reveiw. | 
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.
@ekluzek and I discussed aspects of this PR in detail and Erik posted notes that need to be resolved. Meanwhile though I will approve the PR to avoid delaying the merge to b4b-dev later.
| @@ -0,0 +1,178 @@ | |||
| module CTSMForce2DStreamBaseType | |||
|  | |||
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.
@slevis-lmwg and I went over the basic name and it makes sense to both of us.
I will need to add comments and descriptions, now that the code is working and we think the names are OK.
| type, abstract, public :: ctsm_force_2DStream_base_type | ||
| private | ||
| type(shr_strdata_type), public :: sdat ! Stream data type | ||
| character(len=FL) :: stream_filename ! The stream data filename (also in sdat) | 
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 I really need these here? Can I use the data in sdat?
| real(r8), pointer :: dataptr1d(:) | ||
| integer :: rc ! error return code | ||
|  | ||
| ! Get pointer for stream data that is time and spatially interpolated to model time and grid | 
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 bit about getting the pointer to data could be put in the base type.
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 figured out what to do here in #3566 the data can be directly accessed from here.
| @@ -0,0 +1,193 @@ | |||
| module AtmCarbonIsotopeStreamType | |||
| use shr_kind_mod , only : r8 => shr_kind_r8 | |||
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.
Need to add notes and descriptions and general comments, now that the general structure is approved.
| real(r8), allocatable, private :: atm_delta_c13(:) ! Delta C13 data | ||
| real(r8), parameter :: time_axis_offset = 1850.0_r8 ! Offset in years of time on file | ||
|  | ||
| ! Private data for the control namelist: | 
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 needs to become an actual namelist and added to namelist defaults all the things.
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.
At least the filenames need to be done this way.
Description of changes
Bring in the capability to read in Carbon isotopes in streams. There will be other changes to fully flesh this out. And a version that allows it to be turned on instead of the current file method. The final thing will be a tag on master to change the default to only using streams. And then on b4b-dev we can remove the old version.
Specific notes
Contributors other than yourself, if any:
CTSM Issues Fixed (include github issue #):
Design notes in #3546
Fixes #3544
Fixes #3502
Work on #3346
Are answers expected to change (and if so in what way)? No
Any User Interface Changes (namelist or namelist defaults changes)? Yes
New namelist options for stream handling will come in:
Does this create a need to change or add documentation? Did you do so? No No
Testing performed, if any: Single case testing so far, will run aux_clm
Currently just running an SMS test: SMS_D_Ld3.f10_f10_mt232.IHistClm60BgcCropCrujra.derecho_intel.clm-clm60cam7LndTuningMode
Will transition to the 3-year test starting in Dec/2013 used previously