fix to cm_flex_tax bug

[Renato-Rodrigues]

Purpose of this PR

Aims to fix the problem discussed here: #1115
The proposed solution follows the addition of a non negative pm_SEprice parameter and its use in the q32_flexAdj equation.

Test runs with and without the change can be found here (still running):

• /p/projects/remind/users/renatoro/Debug_trunk/2023_01_11_cm_flex_tax/output/SSP2EU-AMT-Base_2022-12-24_00.08.15
• /p/projects/remind/users/renatoro/Debug_trunk/2023_01_11_cm_flex_tax/output/SSP2EU-AMT-NDC_no_change_2023-01-11_10.33.56

Type of change

• Bug fix
• Fundamental change

Checklist:

• My code follows the coding etiquette
• I have performed a self-review of my own code

Further information:

• There is unnecessary code repetition for the definition of pm_SEprice, currently repeated in multiple places in the model.
• only two assignments are really required: one at the core/preloop, and another in the core/postsolve. Repeating the assignment at every power sector module and at every presolve is quite unnecessary.
• This refactoring was not done in this PR, and an issue was added to mark this refactoring as pending. pm_SEprice assignment repeated unnecessarily all over the model.  #1149

[0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q]

My suggestion at the IEA-Update channel was in this line, but instead of creating an additional parameter, I suggested to add a dollar condition to the right hand side of the equation so it would be zero when prices are negative. Both should theoretically work in the same way.

Why did you go for the additional parameter? One more thing in the .gdx file …

[Renato-Rodrigues]

I could go either way.
I tend to agree with you that using less memory is always better, but others prefer to have "easier to read" code. Dollar conditionals are a bit less straight forward to read to some.

I will adjust the PR to whatever solution the reviewers vote for, so if @LaviniaBaumstark and @robertpietzcker have a preference for the parameter way, I would leave as is. If they are ok with the dollar conditional way, I will make the necessary changes to adapt this PR to that solution instead.

[0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q]

Yeah, but … ☝️ there are dollar-conditions already, so not much is lost I would argue.

[robertpietzcker]

Thanks for the fix, Renato!

It seems I am not yet up to full speed - yesterday I somehow thought using a $ conditional in the multiplication in q32_flexAdj to take out the negative values would somehow cause a problem as it would leave vm_flexAdj free in that case (instead of fixing it to 0), but now I don't understand anymore why I thought so.

I am sorry for the additional hassle!

Please go with the version you prefer, Renato.

[0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q]

It seems I am not yet up to full speed - yesterday I somehow thought using a $ conditional in the multiplication in q32_flexAdj to take out the negative values would somehow cause a problem as it would leave vm_flexAdj free in that case (instead of fixing it to 0), but now I don't understand anymore why I thought so.

Because you confused dollar conditions in assignments with those in equations.

lhs(x)$( CONDITION ) = rhs(x)

will not assign anything to lhs if the condition is negative, while

lhs(x) = rhs(x)$( CONDITION )

will assign zero.

But in equations, left hand side and right hand side are arbitrary. Everything behaves as does rhs above, getting replaced by zero if the condition is negative.

[Renato-Rodrigues]

It seems I am not yet up to full speed - yesterday I somehow thought using a $ conditional in the multiplication in q32_flexAdj to take out the negative values would somehow cause a problem as it would leave vm_flexAdj free in that case (instead of fixing it to 0), but now I don't understand anymore why I thought so.

My original post in the IEA channel had a mistake as the dollar conditional was applied to the equation declaration and not to the right hand side of it. It was probably that version that you look at.
I mentioned that issue just after I wrote the original equation in the same mattermost channel: https://mattermost.pik-potsdam.de/rd3/pl/njtnam3mkfbqdcgemdog6s68yy , but after that I editted the original post to avoid the misleading message.

I will set a run with the dollar conditional alternative and compare the solutions. If everything works the same way I will update this PR and use the dollar alternative instead.

PS: didn't see that Michaja wrote a reply for that just before me :)

[LaviniaBaumstark]

I also prefer the dollar condition. In my first test this led to unbounded variables but probably I did a mistake. So, let's wait for your tests and if it is working, go with the dollar condition

[Renato-Rodrigues]

I changed the solution to use the dollar formulation.
I set a test run at: /p/projects/remind/users/renatoro/Debug_trunk/2023_01_11_cm_flex_tax_dollar/output/SSP2EU-AMT-NDC_dollar_2023-01-11_18.10.15
tomorrow I will check the results to merge this in case of success.

[Renato-Rodrigues]

At least the test runs I tried were successful.
The model solution time also reduced at least half hour due to the need for less iterations. It converged in the minimum number of iterations possible (28 or 29 iterations depending of the run) instead of 38 iterations from the previous successful NDC run.

I still got the issue I described here #1137 in one of the runs although.

I am merging this PR as is, but we still need to verify if that is enough to solve the issue with the marginals use in equations, and if this issue was causing the REF and MEA consumption losses described by Marian in the validation results.
I am not sure about that yet.