Add single-variable time series file generation function from ADF

[nusbaume]

This should hopefully generalize the timeseries generation capability so that any CUPiD script or notebook can use it.

[wwieder]

Is this generic enough for different history file types (e.g. patch or landunit level output from CLM, as opposed to grid cell averages)

[TeaganKing]

@wwieder Are you referring to the hist_str here? This is now an editable quantity in the config.yml file. Would that be sufficient for those other kinds of history file output? I'm not familiar with those kinds of output, so any clarification would be helpful.

[TeaganKing]

Also mentioned in #86 ...

[TeaganKing]

We should implement this by adding a call to run.py under if time_series.

[dabail10]

Is this ready for testing?

[TeaganKing]

Is this ready for testing?

Hi @dabail10 , sorry for the slow response, I'm in full-day training every day this week... I'm hoping to, at the least, wrap this up before our meeting next week, and will remove the draft label once it's ready for testing/review.

[TeaganKing]

I've added the general infrastructure for components to use this timeseries generation. Each component will need to adjust the variables they want to generate (unless processing all vars) and the relevant history string (h0 is the current default) in config.yml. Additionally, in run.py, each component will need to update the height dimension they are using ('lev' is currently the default); this is in run.py instead of the config file since I don't anticipate it changing/needing to be modified by users.

[TeaganKing]

Thanks for those suggestions, @mnlevy1981 ! All of those comments have been addressed.

[mnlevy1981]

I'll run this branch before the 3:00 CUPiD meeting, but these are two more comments from just looking through the code

[mnlevy1981]

Could we update README.md to mention the ability to generate time series files? Relatedly -- when I ran this myself, I didn't have the nco module loaded so I got an error

FileNotFoundError: [Errno 2] No such file or directory: 'ncrcat'

Maybe a quick note in the README that time series generation requires NCO? Or can we install NCO via conda? if so, maybe we should just add it to conda-dev?

[kafitzgerald]

You can certainly install it via conda, but I'm not sure if there are advantages to using the module(s) available on HPC. Potentially some performance / compatibility tradeoffs there.

[TeaganKing]

Ok, I added another note to #86 to investigate any potential tradeoffs with NCO, as well as made a note in the README.

And thanks for the loop suggestion Mike-- definitely a good idea!

[TeaganKing]

I also updated the project vision as I was looking at the README anyways...

[rmshkv]

To elaborate on something I mentioned in #88 - a potential suggestion (that we don't have to take, definitely backseat driving a bit here): the CUPiD/ploomber framework is already set up to support multiple phases of tasks that depend on each other, e.g. running timeseries generation first before diagnostics. I would encourage at some point moving the timeseries config block to be under compute_scripts in config.yml, which is already set up to be organized by components that can be turned on or off, and parameters specific to each component that can be passed in to a universal script. This keeps things more generalizable and less hard-coded. We could then use options in ploomber's tasks to specify that timeseries should be run first. Before that happens we need to document the script functionality (which I'll do soon :) ) and maybe build it out a little more, but after that I'd be happy to make a draft of that modification and see if it's something we want to go with.

[TeaganKing]

To elaborate on something I mentioned in #88 - a potential suggestion (that we don't have to take, definitely backseat driving a bit here): the CUPiD/ploomber framework is already set up to support multiple phases of tasks that depend on each other, e.g. running timeseries generation first before diagnostics. I would encourage at some point moving the timeseries config block to be under compute_scripts in config.yml, which is already set up to be organized by components that can be turned on or off, and parameters specific to each component that can be passed in to a universal script. This keeps things more generalizable and less hard-coded. We could then use options in ploomber's tasks to specify that timeseries should be run first. Before that happens we need to document the script functionality (which I'll do soon :) ) and maybe build it out a little more, but after that I'd be happy to make a draft of that modification and see if it's something we want to go with.

@rmshkv thanks for this suggestion! Just to clarify, are you suggesting creating a new compute_scripts block? I don't see a pre-existing one (although I do see compute_notebooks). Also, maybe it would be best to do this after #88 comes in? I think it would be a fairly simple change to move these and adjust how things are called from the config file in run.py, but I'm a bit hesitant to do that before #88 comes in.

[rmshkv]

To elaborate on something I mentioned in #88 - a potential suggestion (that we don't have to take, definitely backseat driving a bit here): the CUPiD/ploomber framework is already set up to support multiple phases of tasks that depend on each other, e.g. running timeseries generation first before diagnostics. I would encourage at some point moving the timeseries config block to be under compute_scripts in config.yml, which is already set up to be organized by components that can be turned on or off, and parameters specific to each component that can be passed in to a universal script. This keeps things more generalizable and less hard-coded. We could then use options in ploomber's tasks to specify that timeseries should be run first. Before that happens we need to document the script functionality (which I'll do soon :) ) and maybe build it out a little more, but after that I'd be happy to make a draft of that modification and see if it's something we want to go with.

@rmshkv thanks for this suggestion! Just to clarify, are you suggesting creating a new compute_scripts block? I don't see a pre-existing one (although I do see compute_notebooks). Also, maybe it would be best to do this after #88 comes in? I think it would be a fairly simple change to move these and adjust how things are called from the config file in run.py, but I'm a bit hesitant to do that before #88 comes in.

The functionality to handle a compute_scripts block provided by config.yml already fully exists - there just isn't a compute_scripts block in the existing example in this repo. That's why I need to document it, it's kind of a secret feature right now :) And yeah agreed that this and #88 should be merged in first - I just wanted to get my thoughts written down somewhere so we can discuss.

[TeaganKing]

@rmshkv Got it, that makes sense now! Thank you for clarifying! It seems like your suggestion is probably the best way forward, but I summarized this and some other relevant discussions that Mike and I had in #86 (with a note that we should probably use compute_scripts) just so the information is all in one place.

[mnlevy1981]

This looks great! One more minor-ish suggestion, broken into two inline comments (and if we want to push it to #86 that's okay).

[mnlevy1981]

Looks good, great work!

[TeaganKing]

Thanks for all your help and suggestions from everyone who commented!