Skip to content
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

Feature/update cesm driver namelist defaults #800

Conversation

dgergel
Copy link
Contributor

@dgergel dgergel commented May 24, 2018

@dgergel dgergel self-assigned this May 24, 2018
@@ -54,55 +54,59 @@ This is a minor update from VIC 5.0.1. The VIC 5.1.0 includes new features, such
- Fixed bug that prevented using the correct local domain grid cells in `cesm_put_data.c`
- Changed reference temperature units from Celsius to Kelvin in `cesm_put_data.c`

1. [GH#695](https://github.com/UW-Hydro/VIC/pull/695)
2. [GH#695](https://github.com/UW-Hydro/VIC/pull/695)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

markdown automatically numbers this so the 1. was intentional here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @jhamman says. Leave the 1 for all the numbered entries. The 1 just indicates that markdown should format this as an enumerated list and will take care of the actual numbering. It is much easier to leave that to markdown in case the list ever gets updated.

@@ -91,7 +91,8 @@ def copy_to_rundir(grid_config, caseid, rundir, casedocs):

# copy file
copy_clean_vic_config(grid_config[filekey], dst_file,
header=header, rundir=rundir, **grid_config)
header=header, rundir=rundir,
caseid=caseid, **grid_config)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation error here.

OUT_FORMAT NETCDF4_CLASSIC
OUTVAR OUT_PREC
HISTFREQ END
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for both of these streams, I think you probably want the HISTFREQ frequency to be nmonths 1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would HISTFREQ END do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write one file with all history timesteps in it at the final timestep. This is probably not what you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated as suggested

OUTVAR OUT_SURF_TEMP
OUTVAR OUT_SWNET
OUTVAR OUT_LWNET
OUTVAR OUT_GRND_FLUX
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest adding frozen soil variables. You and @bartnijssen should probably iterate together on which variables you think would be potentially useful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having something that let's us identify active layer depth would be useful, but now sure whether that would be part of every run. Same for all these met variables. Can we split out the radiation variables (down and up rather than just net).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I was thinking too. From knowing how much outputting data for 10 thermal nodes increases the size of the output files, I think we should add that when we need it and not make it the default. I'll split out the radiation variables and remove some of the met ones

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember this is just a monthly output file so the cost of writing out 3d vars is much less than it would otherwise be. My thought is that you should write out more variables, unless you have reason to think the data volumes will be out of control or that it is negatively impacting runtimes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added soil node temperature to the monthly output files as well as eliminating some of the met vars and splitting out the radiation vars. @bartnijssen and @jhamman - what do you think of the updated ones?

@@ -54,55 +54,59 @@ This is a minor update from VIC 5.0.1. The VIC 5.1.0 includes new features, such
- Fixed bug that prevented using the correct local domain grid cells in `cesm_put_data.c`
- Changed reference temperature units from Celsius to Kelvin in `cesm_put_data.c`

1. [GH#695](https://github.com/UW-Hydro/VIC/pull/695)
2. [GH#695](https://github.com/UW-Hydro/VIC/pull/695)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @jhamman says. Leave the 1 for all the numbered entries. The 1 just indicates that markdown should format this as an enumerated list and will take care of the actual numbering. It is much easier to leave that to markdown in case the list ever gets updated.

OUT_FORMAT NETCDF4_CLASSIC
OUTVAR OUT_PREC
HISTFREQ END
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would HISTFREQ END do?

OUTVAR OUT_SURF_TEMP
OUTVAR OUT_SWNET
OUTVAR OUT_LWNET
OUTVAR OUT_GRND_FLUX
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having something that let's us identify active layer depth would be useful, but now sure whether that would be part of every run. Same for all these met variables. Can we split out the radiation variables (down and up rather than just net).

@dgergel
Copy link
Contributor Author

dgergel commented Aug 24, 2018

@bartnijssen @jhamman just noticed this didn't get merged before - can one of you go ahead and merge it?

@@ -91,7 +91,8 @@ def copy_to_rundir(grid_config, caseid, rundir, casedocs):

# copy file
copy_clean_vic_config(grid_config[filekey], dst_file,
header=header, rundir=rundir, **grid_config)
header=header, rundir=rundir,
caseid=caseid, **grid_config)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you fix the indentation of this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -91,7 +91,8 @@ def copy_to_rundir(grid_config, caseid, rundir, casedocs):

# copy file
copy_clean_vic_config(grid_config[filekey], dst_file,
header=header, rundir=rundir, **grid_config)
header=header, rundir=rundir,
caseid=caseid, **grid_config)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this still looks funny 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh it didn't look weird on my end; I think it's fine now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants