b4b-dev: Move nml parameters (changes paramfile)#3391
b4b-dev: Move nml parameters (changes paramfile)#3391slevis-lmwg merged 14 commits intoESCOMP:b4b-devfrom
Conversation
|
I did a quick skim over this and this looks good to me. I'll rebase this PR to go to b4b-dev, which should be fine without any trouble. Sometimes you have to rebase the branch when that's done, but I don't expect that now. The one thing noticeably missing is an updated parameter file with the parameters moved to it. |
|
Adding next to talk about who will shepherd this in. |
|
parameter file location: /fs/cgd/csm/inputdata/lnd/clm2/paramdata/ctsm5.3.041.Nfix_params.v13.c250221_upplim250_add_nml.nc |
|
We talked about this and will bring it in on b4b-dev. |
ekluzek
left a comment
There was a problem hiding this comment.
I had looked at this when this first came in and thought I had already approved. So approving now.
I skimmed through again and didn't notice anything that warrants attention.
|
Paused testing because need to bring the new params (see post above) to the latest paramfiles: |
|
Before I add the new params to paramfiles:
ctsm60 and clm50 are the paramfiles to update, not clm45. Consider the new tool in #3622 if needed. |
|
I generated two new paramfiles with the updates:
|
|
Ran testing on Derecho last night. Things worked as expected with the exception of the following: I think this is mostly because of tests that have custom parameter files. |
|
the original code mixes units, and I thought it was more clear to add the
suffix to the variable name.
…On Thu, Mar 5, 2026 at 11:30 AM Erik Kluzek ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/main/atm2lndType.F90
<#3391 (comment)>:
> - ' ERROR: For repartition_rain_snow true, precip_repartition_glc_all_snow_t must be provided')
- end if
- if (.not. present(precip_repartition_glc_all_rain_t)) then
- call endrun(subname // &
- ' ERROR: For repartition_rain_snow true, precip_repartition_glc_all_rain_t must be provided')
- end if
- if (.not. present(precip_repartition_nonglc_all_snow_t)) then
- call endrun(subname // &
- ' ERROR: For repartition_rain_snow true, precip_repartition_nonglc_all_snow_t must be provided')
- end if
- if (.not. present(precip_repartition_nonglc_all_rain_t)) then
- call endrun(subname // &
- ' ERROR: For repartition_rain_snow true, precip_repartition_nonglc_all_rain_t must be provided')
- end if
+ ! non-glacier all rain temperature (degrees C)
+ call readNcdioScalar(ncid, 'precip_repartition_nonglc_all_rain_t', subname, precip_repartition_nonglc_all_rain_t_celsius)
@swensosc <https://github.com/swensosc> question here. This is reading in
the variable from the params file without the suffix "_celsius" in the name
-- but putting it into a variable with that suffix. This is something that
can be confusing when variables have a different name than the variable
name on input files.
—
Reply to this email directly, view it on GitHub
<#3391 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGRN57GKF3BRTUOPV2BNZOT4PHBVLAVCNFSM6AAAAACDLEK2KCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTQOJYG43DSNZWHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I have picked this up again hoping to merge this week if possible. I made changes to the constructor as I understood them from the conversation above. comparing to baseline ctsm5.4.021. FUNIT still fails at RUN. @ekluzek I will push my mods to get your feedback. UPDATE 2026/3/11 after the mods discussed below: So I will push my mods and submit aux_clm. |
This is in atm2lndType.F90 and FUNITCTSM test still fails at RUN.
…he namelist move for ESCOMP#3391 the move_nml_parameters branch
|
|
|
@ekluzek please look if you want to go over my latest changes. Tests are passing now, including FUNIT, so I submitted aux_clm. I'm not confident about one variable naming choice: I kept Sean's |
Description of changes
move namelist parameters to parameter file. parameters moved are: baseflow_scalar, precip_repartition_nonglc_all_rain_t, precip_repartition_nonglc_all_snow_t, precip_repartition_glc_all_rain_t, precip_repartition_glc_all_snow_t
Specific notes
Contributors other than yourself, if any:
CTSM Issues Fixed (include github issue #):
Are answers expected to change (and if so in what way)? No
Any User Interface Changes (namelist or namelist defaults changes)? Yes
Does this create a need to change or add documentation? Did you do so? No No
Testing performed, if any:
@slevis-lmwg working locally in
/glade/work/slevis/git/b4b-dev