search

CH-Earth / summa

Structure for Unifying Multiple Modeling Alternatives:
http://www.ral.ucar.edu/projects/summa
GNU General Public License v3.0
80 stars 105 forks source link

1) write hruId to all output files (using same function with hru) #377

Closed andywood closed 4 years ago

andywood commented 4 years ago

2) avoid opening and default-writing timestep variables 3) remove pre-loaded outputfile suffixes 4) miscellaneous typo fixes

andywood commented 4 years ago

please take a look & think about whether these changes are desired & done in the best way. I think having hruId in the output files makes a lot of sense when we run split domains and need to merge files; also not writing timestep files unless they're needed will make the model lighter for some apps. Not forcing the user to have 'output' or 'spinup' in output file names is a matter of preference. Typically all my output paths already have the word 'output' somewhere so I tend to leave it out of the actual output filename if I can ... a VIC-style user choice. I'm not sure what the 'spinup' means wrt water year files but that's also now a choice that can be put in the filemanager. If these changes are acceptable there are a few compilation warnings that could be cleaned up; and I left commented out lines in the code for ease of re-adding them. They'd be taken out and I'd generate a new clean PR.

martynpclark commented 4 years ago

The code looks good to me. Once you get comfortable with what you've done it would be good to remove some of the lines that are commented out.

Have you tested this yet?

andywood commented 4 years ago

I've been running this code and it works fine. I did test in addition to writing timestep-level files that were the defaults before (eg with nSnow, nSoil etc), and it worked. I'll make a clean PR. Thanks for looking through this.