Closed l-kotzur closed 5 years ago
Hi Leander, thank you for suggesting to include this very important feature! A few things I noticed at a first glance:
Hey Lara,
thanks for your feedback!
It would be nice to include a readme such that users who want to test their pull request before hand know how to do that. This would be nice to have in the test folder itself but also in the readthedocs documentation (which can of course be linked).
To test it locally, it is only necessary to call from the command line:
pytest
Could you or @t-gross decide where to add it to the documentation?
Maybe it would be worthwhile to consider if we also want to include geopanads in the dependencies for the testing? I have mixed feeling about this, but if we want to include testing our output plotting it might be worthwhile to consider.
This should be done in case that the output testing is included as well. It should be easily possible via conda install.
The water supply example is a quite simplistic example which does not cover so many input parameters and variations of them. I think it is a good example for a first start, but maybe we should also try to include more complex test examples in the future?
I thinks it would be great to add further examples. Nevertheless, the test cases should solve fast without the requirements of commercial solvers.
I would prefer if we do not include this branch right away with the develop branch but first as a thematical branch. In this way we could maybe first add some more testing examples before we merge it with the develop? And it would give the other collaborators the chance to play around with it a bit more. But that's of course open for discussion! Nevertheless, I think it would be important to include this feature fast!
Done! :)
Nice (: I talked to @t-gross and she will add the required documentation!
Concerning installing geopandas using conda install: In the newer Python version this sometimes fails and you have to manually install the dependencies of geopandas. But we can have a closer look at that when we come to this point. I think adding the output testing right away (or at least some parts of it) would be something to consider.
Overall, I guess we should all sit together in the near future and discuss what we want to have included in the testing.
Otherwise, I am super okay with pulling this branch to the testing branch. Again, thank you very setting this up. @t-gross @h-heinrichs ?
I have included automized testing via Travis. It checks the package compability of FINE, the main interface syntax and furhter tests if the Water Supply Example solves to the same values as in the previous FINE versions. It shall be all deterministic since you defined a seed for numpy and an hierarchical aggregation is used in the time series aggregation.
Nevertheless, I needed to adapt the Water Supply Example, since Gurobi and GLPK solved to the same objective value but different operation values for the original Example, hinting to a flat solution space. I modified the parameters of the water supply example such that a single optimal solution occurs for both solvers.