-
Notifications
You must be signed in to change notification settings - Fork 321
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
improve ui for neon script #1467
Conversation
Jim, this looks good. The only thing that may be helpful is some comments in the code, help functions, or a README that explains what the workflow and how users should use this script. |
[default: %(default)s] | ||
''', | ||
choices = ["ad", "postad", "transient", "sasu"], | ||
default = "transient") |
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 like the implementation of specifying the run_type here better than how it was before. Thanks for that.
My only concern is the options that are specific for each run type... If we can generalize the options enough so it works for all run_types, I like this implementation much better.
Thanks @jedwards4b. The changes look great. This will be part of the README for our future use. |
When I am running the code, I receive the following error without any meaningful help:
Again, I think this looks good. |
@@ -19,7 +19,7 @@ | |||
!---------------------------------------------------------------------------------- | |||
|
|||
flanduse_timeseries = ' ' ! This isn't needed for a non transient case, but will be once we start using transient compsets | |||
fsurdat = "$DIN_LOC_ROOT/lnd/clm2/surfdata_map/NEON/surfdata_hist_78pfts_CMIP6_simyr2000_${NEONSITE}_c210720.nc" | |||
fsurdat = "$DIN_LOC_ROOT/lnd/clm2/surfdata_map/NEON/surfdata_hist_78pfts_CMIP6_simyr2000_${NEONSITE}_c210805.nc" | |||
model_year_align_urbantv = 2018 | |||
stream_year_first_urbantv = 2018 | |||
stream_year_last_urbantv = 2019 |
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.
Don't we want ndep and popdens to update to the last year of the simulation (or at least the spinup)? this would be 2020 for spinup and 2021 for transient cases. How do we keep updating this moving forward?
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.
@jedwards4b do these get updated somehow 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.
@wwieder these don't get autoupdated by any means. Right now the only way to change it is to keep changing the last year in this file (or in each case run). The last year update could also be handedl in run_neon.py though. I don't know that's any better though.
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 think this is fine for now, but as we move to making more of the real-time hindcasts they should be updated for new years of data? Updating urban likely ins't too important and for short runs the deposition files shouldn't changethat much?
@wwieder added some notes about leap years. The model CAN run with leap years when flipped to GREGORIAN calendar mode. Just need to
This could be added as a default setting for NEON sites. I actually thought we already were doing that, but it looks like we aren't yet. Should that be done? |
|
@@ -4254,7 +4254,7 @@ sub check_input_files { | |||
my $pathname = $nl->get_variable_value($group, $var); | |||
# Need to strip the quotes | |||
$pathname =~ s/['"]//g; | |||
|
|||
next if ($pathname =~ /UNSET$/); |
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 also did work in 017a7ed to prevent blank files being listed. I think @jedwards4b is also a good protection although I think it really should trigger an error because something is screwy if you have a namelist item set to UNSET.
The comment that generated this was...
@@ -1,3 +1,4 @@ | |||
./xmlchange NEONSITE=GUAN | |||
./xmlchange PTS_LON=293.13112 | |||
./xmlchange PTS_LAT=17.96882 | |||
./xmlchange RUN_STARTDATE=2018-06-01 |
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.
@wwieder It will just start on June/1st and use that as it's reference date. This means for purposes of spinup dynamics June/1st is the date when spinup variables are updated. It's sort of like making June/1st the start of your fiscal year. Somethings are still tied to the calendar year (like dynamic land use change), but anything that is measuring time since start of run or doing running averages will use June/1st as the marker. Does that help?
@@ -19,7 +19,7 @@ | |||
!---------------------------------------------------------------------------------- | |||
|
|||
flanduse_timeseries = ' ' ! This isn't needed for a non transient case, but will be once we start using transient compsets | |||
fsurdat = "$DIN_LOC_ROOT/lnd/clm2/surfdata_map/NEON/surfdata_hist_78pfts_CMIP6_simyr2000_${NEONSITE}_c210720.nc" | |||
fsurdat = "$DIN_LOC_ROOT/lnd/clm2/surfdata_map/NEON/surfdata_hist_78pfts_CMIP6_simyr2000_${NEONSITE}_c210805.nc" | |||
model_year_align_urbantv = 2018 | |||
stream_year_first_urbantv = 2018 | |||
stream_year_last_urbantv = 2019 |
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.
@wwieder these don't get autoupdated by any means. Right now the only way to change it is to keep changing the last year in this file (or in each case run). The last year update could also be handedl in run_neon.py though. I don't know that's any better though.
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.
These changes look good
@ekluzek I have one outstanding question about how the model handles spinup when data are not available Jan 1 for a particular year, but otherwise I think my questions have been addressed for this PR. |
@wwieder consider the log file here: /glade/scratch/jedwards/NEON/archive/GUAN.ad/logs/atm.log.290175.chadmin1.ib0.cheyenne.ucar.edu.210901-142537 It starts the spinup at model date 180101 and with ATM data file GUAN_atm_2020-01.nc and spins up using 2019 and 2020 data only. This is controlled by DATM_YR_START which is set via variable self.start_year in the run_neon.py script using the first year for which JAN 1 data is available. |
thanks for clarifying @jedwards4b |
… script and exclude the pytlint file, also have pylint ignore an option that black uses so they are compatable
sorry, one more on these lines
Should all of these at least go to 2020 for the spinups, which is what we're doing now? They'll get out of date for the transient runs, but I'm less worried about this (for now), as it will be a while untile we have many years of data past 2020 |
@wwieder do all sites have data that goes through 2020 now? The would be problems with cases that don't have 2020 data as the case will point out the missing a files. Bit that could be considered a feature not a big... |
Yep we're (mostly) spinning all sites up by cycling over 2018-2020 data and then running a full 'transient' from 2018-2021 |
…to jedwards/neon_updates
… a config file for it
…n error code up the chain and the code can die rather than going on as if an error didn't happen
…negative longitude this fixes ESCOMP#1574
Description of changes
Redo options list to remove positional arguments that were difficult to input correctly.
Transient runs now use run_type startup and get finidat from s3 server unless --run-from-postad option is used (or finidat is not available). Use mpi instead of mpi-serial, this mod was recommended for container use. Add a new script neon_finidat_upload which allows authorized users to upload finidat files to the s3 server.
Specific notes
Contributors other than yourself, if any: @ekluzek
CTSM Issues Fixed (include github issue #):
Fixes #1563
Fixes #1550
Fixes #1574
Are answers expected to change (and if so in what way)? No
Any User Interface Changes (namelist or namelist defaults changes)? Yes for NEON
Testing performed, if any: regular testing and tools testing will be done
cheyenne: -- PASS
izumi: -- OK