rl-institut / multi-vector-simulator

Multi-vector Simulation Tool assessing and optimizing Local Energy Systems (LES) for the E-LAND project
GNU General Public License v2.0
21 stars 10 forks source link

Fix missing constraints error #845

Closed Bachibouzouk closed 3 years ago

Bachibouzouk commented 3 years ago

Fix #827

Changes proposed in this pull request:

The following steps were realized, as well (if applies):

For more information on how to contribute check the CONTRIBUTING.md.

smartie2076 commented 3 years ago

834 changed D2 majorly.

Bachibouzouk commented 3 years ago

I think for the constraints it would make sense not to have constraints, rather than having default values to them.

Bachibouzouk commented 3 years ago

834 changed D2 majorly.

I could reapply my changes on top, the gist of it is to make sure the code runs if the constraints are not defined (also good for back compatibility ;))

smartie2076 commented 3 years ago

As you say that this will make sure that MVS stays downward compatible, lets use this PR to evade some issues. I would say that it does not really fix the issue with the parameters, but we have two similar issues anyway (#780 and #827), so one of them can be closed with this.

Bachibouzouk commented 3 years ago

The error (missing Key in the last test of test_D0) originate from a part of code which was in dev, @ciaradunks do you know how it should be fixed?

smartie2076 commented 3 years ago

Hi @Bachibouzouk! I think the issue is that now, with your changes, you do your pre-processing with D0.prepare_constraint_minimal_renewable_share() first and then evaluate the constraint. Was this intentional?

Basically, before that the test was able to skip the constraints and run the test on tests/test_data/inputs_for_D0/mvs_config.json. There, energyVector is missing from the evaluated energyProduction consumption source:

grafik

So, you can either add an energyVector to this, or you move the check for the value of the constraint before D0.prepare_constraint_minimal_renewable_share(), if that is possible.

This is a reason why I think that D0, D1 would do better if we would not test them with a json file but with input csv files...

Bachibouzouk commented 3 years ago

I think you should rather move the pre-processing into if-loop. It seems that would not cause any problems.

Are you intentionally not writing with functions you changed in the CHANGELOG? Especially that the validity checks are only performed if a constraint is actually added is also useful information.

Not writing the function names in the CHANGELOG was not intentional, merly laziness I would guess, I will update when I fix