Add marrmot M01 model class #56
Conversation
Commits to PR will trigger CI job via synchronize event type
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
So no installation of any deps is needed.
There was a problem hiding this comment.
Nice work @SarahAlidoost and @sverhoeven !
The get_value_as_xarray function seems to only work on flux_out_Q. With both P and T it crashes. Is this "future work"? :-)
Other than that good to go, so feel free to merge (perhaps add an issue on the limited use of get_value_as_xarray)
| """ | ||
| if work_dir is None: | ||
| scratch_dir = CFG['output_dir'] | ||
| # TODO this timestamp isnot safe for parallel processing |
There was a problem hiding this comment.
This is going to be a problem quickly. Perhaps add milliseconds to kick the can down the road?
There was a problem hiding this comment.
When I was working with marrmot calibration and parallel processing, I found that this solution does not work because the CPU time would be the same for all cores. We added a random filename generator there.
|
Sonarcloud is complaining about duplicate lines in test code. I guess we should either fix or ignore test code in quality verification. @sverhoeven do you know what the approach usually is? |
Thank you @nielsdrost. I made issue eWaterCycle/ewatercycle-marrmot#6 and added a TODO and |
Sonarcloud does not report duplication in test code only in ewatercycle/models/marrmot.py. I would not change the gate, but do some work after EGU to lower code duplication and increase coverage. The get_value_as_xarray method should be moved to a super class, as all models we currently have can use same method. However later we will get some models that will not be able to return xarray due to their grid type. So we might need a sub class of AbstractModel to handle implement get_value_as_xarray and the models can be subclasses of that one. |
|
SonarCloud Quality Gate failed. |
No description provided.