Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

restrict definition of q37_feedstockShares #1599

Merged
merged 8 commits into from
Mar 7, 2024

Conversation

mellamoSimon
Copy link
Contributor

@mellamoSimon mellamoSimon commented Mar 5, 2024

Purpose of this PR

restrict the definition of q37_feedstockShares to relevant sets.

Type of change

  • Bug fix

Checklist:

  • My code follows the coding etiquette
  • I performed a self-review of my own code
  • I explained my changes within the PR, particularly in hard-to-understand areas
  • I checked that the in-code documentation is up-to-date
  • I adjusted the reporting in remind2 where it was needed
  • I adjusted forbiddenColumnNames in readCheckScenarioConfig.R in case the PR leads to deprecated switches
  • All automated model tests pass (FAIL 0 in the output of make test)
  • The changelog CHANGELOG.md has been updated correctly
  • Test runs are here:
    /p/tmp/simonlei/this-is-remind/remind/output/ and here /p/tmp/pehl/Remind/output/SSP2EU-NPi_2024-03-06_11.31.54 with a working input gdx.

@mellamoSimon
Copy link
Contributor Author

test runs are running in the above mentioned folder

Copy link
Member

@Renato-Rodrigues Renato-Rodrigues left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks fine so far from the NPi run at:
/p/tmp/simonlei/this-is-remind/remind/output/SSP2EU-NPi_2024-03-05_13.38.14

I would just resolve the conflicts (capitalization of some sets) and I would be fine with the merge.

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member

@mellamoSimon, do mellamoSimon#8.
What is the purpose of q37_FossilFeedstock_Base?

@mellamoSimon
Copy link
Contributor Author

What is the purpose of q37_FossilFeedstock_Base?

that was set up by @fschreyer to prevent the model from using biomass or synfuels in baseline runs. However, I think that now that might be unnecessary because we improved emissions accounting (including incineration) and we are not supposed to use baseline anymore...

@mellamoSimon
Copy link
Contributor Author

I'm halting this merge, cause the tests failed for NPi

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member

I'm halting this merge, cause the tests failed for NPi

👉 mellamoSimon#8

@mellamoSimon
Copy link
Contributor Author

I strated new tests in the same folder

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member

The current REMIND input gdxes are thoroughly broken, so I started a second test with the previous input gdx (c1256714220e99250f791cc35e61ae51d3bdc7da) at /p/tmp/pehl/Remind/output/SSP2EU-NPi_2024-03-06_11.31.54.

@mellamoSimon
Copy link
Contributor Author

Quick update as Michaja is out of the office for the rest of this week. Below is the summary for this run:

The current REMIND input gdxes are thoroughly broken, so I started a second test with the previous input gdx (c1256714220e99250f791cc35e61ae51d3bdc7da) at /p/tmp/pehl/Remind/output/SSP2EU-NPi_2024-03-06_11.31.54

So I'm not sure if I should merge yet. Maybe a new calibration will fix things but I don't know about the workflow in this case: should we re-calibrate and test first and then merge? Thanks!

REMIND run finished!

Model summary:
  gams_runtime is 12.9 hours.
  full.gms exists, so the REMIND GAMS code was generated.
  full.log and full.lst exist, so GAMS did run.
! abort.gdx exists, a file containing the latest data at the point GAMS aborted execution.
  non_optimal.gdx exists, because iteration 69 did not find a locally optimal solution. modelstat: 6 (Intermediate Infeasible)
  fulldata.gdx exists, because iteration 70 was successful. modelstat: 2 (Locally Optimal)
  Modelstat after 70 iterations: 2 (Locally Optimal)
  full.log states: *** Status: Execution error(s)

Infeasibilities extracted from full.lst with nashstat -F:
itr 	 region 	 solvestat            	 modelstat               	 resusd   	 objval           
  6 	 CHA 	    NORMAL COMPLETION 	      LOCALLY INFEASIBLE 	  544.922 	  64.0247890145572 	 F
... 3 infeasibilities omitted, show all with nashstat -a ...
 69 	 MEA 	 TERMINATED BY SOLVER 	 INTERMEDIATE INFEASIBLE 	 1038.762 	 -37173.3285338495 	 F
If infeasibilities appear some iterations before GAMS failed, check nashstat -a carefully.
The error that stopped GAMS is probably not the actual reason to fail.

Failed markets in fulldata.gdx:
 - peoil: 2005, 2010, 2015, 2020, 2025, 2030, 2035, 2040, 2045, 2050 ...
 - pegas: 2005, 2010, 2015, 2020, 2025, 2030, 2035, 2040, 2045, 2050 ...
 - pecoal: 2005, 2010, 2015, 2020, 2025, 2030, 2035, 2040, 2045, 2050 ...
 - good: 2005, 2010, 2015, 2020, 2025, 2030, 2040, 2045, 2050, 2055 ...
No failed tax convergence in fulldata.gdx.

@Renato-Rodrigues
Copy link
Member

The change in this PR fixes a clear bug that is currently active in the REMIND code.
I would say that if the changes do not cause pre-triangular issues, and if in the runs all industry equations are limited to the right dimensions, we will be in a better shape to figure out which other issues are holding back the REMIND convergence with this merged than not.
So I would say, go ahead and merge the changes here, as long as the points I raised above have no issue.

@mellamoSimon mellamoSimon marked this pull request as ready for review March 7, 2024 12:23
@mellamoSimon
Copy link
Contributor Author

[ FAIL 0 | WARN 0 | SKIP 6 | PASS 80 ] :)

@mellamoSimon mellamoSimon merged commit 94339f9 into remindmodel:develop Mar 7, 2024
2 checks passed
@mellamoSimon mellamoSimon added the Chemicals Collection of PRs relevant for our new colleague modeling the chemical industry label Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chemicals Collection of PRs relevant for our new colleague modeling the chemical industry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants