bugfixes for more robust feasibility #744
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,5 +6,5 @@ | |
| *** | Contact: [email protected] | ||
|
|
||
| vm_landdiff.scale = 10e3; | ||
| v10_balance_positive.scale(j,land_from) = 10e-7; | ||
| v10_balance_negative.scale(j,land_from) = 10e-7; | ||
|
There was a problem hiding this comment. Can you explain what is happening here? Why does it help to switch to 10^-7? There was a problem hiding this comment. I ran There was a problem hiding this comment. Okay this may be more of an understanding question, so the scaling tells gams the order of magnitude of step lengths when iterating towards a local optimum? There was a problem hiding this comment. https://www.gams.com/latest/docs/S_CONOPT4.html#CONOPT4_SCALING |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,6 @@ | |
| *** | MAgPIE License Exception, version 1.0 (see LICENSE file). | ||
| *** | Contact: [email protected] | ||
|
|
||
| vm_secondary_overproduction.scale(i,kall,kpr) = 10e-4; | ||
| vm_cost_processing.scale(i) = 10e5; | ||
| vm_processing_substitution_cost.scale(i) = 10e4; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,4 +5,6 @@ | |
| *** | MAgPIE License Exception, version 1.0 (see LICENSE file). | ||
| *** | Contact: [email protected] | ||
|
|
||
| vm_cost_prod_crop.scale(i,factors) = 10e5; | ||
| v38_investment_immobile.scale(j,kcr) = 10e3; | ||
| v38_capital_need.scale(j,kcr,mobil38) = 10e3; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,7 @@ else | |
| ); | ||
| p44_bii_lower_bound(t2,i,biome44)$(p44_bii_lower_bound(t2,i,biome44) >= 1) = 1; | ||
| p44_bii_lower_bound(t2,i,biome44)$(m_year(t2) < s44_start_year) = 0; | ||
| p44_bii_lower_bound(t2,i,biome44)$(sum(cell(i,j), f44_biome(j,biome44)) <= 1e-10) = 0; | ||
|
There was a problem hiding this comment. Why is this change (and other similar changes where 0 is replaced with 1e-10) necessary? There was a problem hiding this comment. In this specific case for There was a problem hiding this comment. I'm not convinced. Replacing small values with 0 / treating zeros the same as tiny numbers seems like losing control. We don't quite know why this works, but if we replace it with zero it works, and it's almost zero anyway... I have a bad feeling about this practice, we are piling up ultra hard to debug problems in our code this way is my feeling. This is of course more a general comment and not specific to this instance/PR. There was a problem hiding this comment. I don't understand this point. Why should it be an issue to treat tiny numbers like zero? There was a problem hiding this comment.
It's just mathematically wrong, and we don't understand the implications. This lack of control what's going on is what's bugging me, but that is of course inherent to using gams/conopt4, so not something we can change easily :/
That is not a great argument and just makes me feel more anxious.
Unfortunately no, I cannot follow what is happening really, so I'll approve. Wanted to emphasize my concerns though. There was a problem hiding this comment. Would be interested in the other reviewers perspective on this @tscheypidi @FelicitasBeier @k4rst3ns :) Am I just worrying to much? 🙈 There was a problem hiding this comment. I totally get your concern. It has been a common practice in the past, though. So maybe something to discuss in the next MAgPIE meeting? There was a problem hiding this comment. Same concern here. On Florians:
... I would not know why theoretically and generally this should be an issue. But I am also see @flohump point: this is not specific to this PR. And I like @FelicitasBeier suggestion bringing this up to the MAgPIE meeting and approving this for now. |
||
| * The lower bound of `v44_bii` is set to `p44_bii_lower_bound` to avoid a reduction of BII in combination with `v44_bii_missing`. | ||
| v44_bii.lo(i,biome44) = p44_bii_lower_bound(t,i,biome44); | ||
| display p44_bii_lower_bound; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,6 @@ | |
| *** | MAgPIE License Exception, version 1.0 (see LICENSE file). | ||
| *** | Contact: [email protected] | ||
|
|
||
|
|
||
| vm_cost_prod_livst.scale(i,factors) = 10e5; | ||
| vm_cost_prod_fish.scale(i) = 10e5; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that
vm_land.lshould suppose to be the same as pcm_land.But as you also pointed out pcm_land is actually changed in some resolve functions, so this is not at all identical or not?
I am not 100% sure why we need this balancing, if we could just define a variable with a level that has the correct value of pcm_land?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this instance it does not make a difference if we use
pcm_landorvm_land.l. They should be identical.The point is that
vm_land.lis changed in the presolve based on parameters, which seem to have a different accuracy than the variable.I tried without the balancing variables. But without them some of the test runs are infeasible.