SIP78 Convert switches to run time options part 3#114
Conversation
…ms in sipnet.in, again
dlebauer
left a comment
dlebauer
left a comment
There was a problem hiding this comment.
Very impressive PR - this is a major enhancement that will make SIPNET much more user friendly, make it easier to compare different model formulations for both testing and theoretical inquiry.
Regarding obsolete parameters - some of my earlier comments may themselves be obsolete. Based on our Slack conversation, the next steps are to remove:
- obsolete parameters remove them from the code, and treat them the same as we do for sipnet.in: if we hit a param we don't know about, give a warning and continue on..
- unused variable
soilWetnessfrom .clim file - this will change the format of the weather (.clim) file and require changing the function that reads climate data.
These tasks can be written up in an issue and set to low priority, perhaps labeled 'good first issue'.
| When adding to the `long_options` struct in step 3(i), give an int value as the last element of the the new | ||
| struct. This is the index that should be used in the `parseCLIArgs()` switch statement for handling the new | ||
| option. The int value must be unique among the other case labels. | ||
| ## Function/Macro Reference |
There was a problem hiding this comment.
It seems like a sentence could help orient the reader, e.g. 'When adding a new CLI option, these functions and macros should be used to ...' perhaps also a brief description under each function or macro.
| // For convenience | ||
| #define FILENAME_MAXLEN CONTEXT_CHAR_MAXLEN | ||
|
|
||
| // 3? Do we need an upper bound? |
There was a problem hiding this comment.
It is reasonable to set an upper bound because this constrains model's structure to what has already been published. I don't think it is necessary to set this bound if the model can scale to >3 pools, but at the same time it isn't a priority at this point, and would be easy enough to change this setting in the future.
|
Issue created: #124 |
…123) * Update docs/parameters.md to reflect deprecated and obsolete params * documented run-time arguments
3 participants
This PR:
soilBreakdown()required byLITTER_POOLsrc/sipnet/sipnet.into show all possible options in that fileMost of the changed lines are from replacing two of the smoke test expected output files. Files of interest for reviewing:
docs/*src/*tests/smoke/run_smoke.shtests/smoke/*/sipnet.inNote: in that "removes additional dead code" bucket, the following variables/params/functions got trimmed. The TrackerVars get set but are never accessed outside of that. I suspect that these were used by functions/modes that themselves have been removed. I did not remove obsolete
params, as I worry that will cause I/O problems with sipnet clients; they have been marked as obsolete in the code.TrackerVarsfa,fr,plantWoodC, andLAIsimpleWaterFlow,transSoilDrainage