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

fill the demSeOth with zeroes so that oneRegi tests work #637

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

lecfab
Copy link
Contributor

@lecfab lecfab commented Aug 6, 2024

While doing make test on Remind, the one-region run gave the following error:

  Differences from first mbind() input:
  spatial:
      missing: `LAM`, `OAS`, `SSA`, `NEU`, ...
       having: `EUR`
  data:
      missing: `SE (EJ/yr)`, `SE|Biomass (EJ/yr)`, `SE|Electricity (EJ/yr)`, `SE|Electricity|Combined Heat and Power w/o CC (EJ/yr)`, ...
       adding: `SE|Input|Hydrogen|Other Energy System Consumption (EJ/yr)`

It is solved by filling v_demSeOth with zeroes where there is no value.

@lecfab lecfab requested a review from orichters August 6, 2024 13:16
@orichters
Copy link
Contributor

Hmm. That surprises me. I know that @fbenke-pik was fighting quite hard against this filling of zeros, I think because it massively blows up memory use. What changes did you do to REMIND before you ran make test? Might there be there problem?

@lecfab
Copy link
Contributor Author

lecfab commented Aug 6, 2024

Ah? I assumed it was ok because of the documentation of readGDX saying:

restoreZeros: Defines whether 0s, which are typically not stored in a gdx file, should be restored or ignored in the output. By default they will be restored. If possible, it is recommended to use restore_zeros=TRUE. It is faster but more memory consuming. If you get memory errors you should use restore_zeros=FALSE

It's linked to the removal of 32_power realizations, I'm trying to merge this: remindmodel/remind#1769

Copy link
Contributor

@orichters orichters left a comment

Choose a reason for hiding this comment

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

I tried to look through your PR but couldn't find any reason that was well-defined before and not now. As noone is around (Michaja, Falk) that is an expert on these memory-related issues and make test seems to work for you with this change, I approve it.

After you ran make test, can you please also run Rscript output.R comp=F output=reporting outputdir=output/testOneRegi and check whether that works with the normal memory configuration?

Would be great if you could check back with @fbenke-pik once he is back from holidays.

@lecfab lecfab merged commit b04d9b6 into pik-piam:master Aug 6, 2024
2 checks passed
@lecfab lecfab deleted the fill-zeroes branch August 6, 2024 14:32
@fbenke-pik
Copy link
Contributor

fbenke-pik commented Aug 12, 2024

@lecfab Oliver is right, restoring zeros should not be the way to solve this (It might still be fine in this case if the data object is not overly large). The sustainable solution is fixing the problem by using the magclass helper matchDim to address dimension mismatches directly (rather than hoping they do not happen by restoring zeros).

Please revert this change in remind2 and provoke the REMIND error again by running "make test". Then fix it in reportSE using matchDim. I am happy to assist, once I can look at a reproducible error.

@orichters Thanks for flagging this.

@orichters
Copy link
Contributor

@fbenke-pik: Sorry, I forgot to mention here that this is exactly what I suggested him later when it came to my mind, see #639

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member

I tried to look through your PR but couldn't find any reason that was well-defined before and not now.

@orichters It was never really "well defined". The testOneRegi gdx would just grandfather-in the values for all the other regions from the input.gdx, so there would be some (meaningless) data there. But as vm_demSeOth was deleted and v_demSeOth introduced (GAMS does not know about "renaming"), and Fabrice started the tests from an old gdx, there was no data in v_demSeOth other than that for EUR.

After you ran make test, can you please also run Rscript output.R comp=F output=reporting outputdir=output/testOneRegi and check whether that works with the normal memory configuration?

Memory overhead for the bunch of zeros is 96 MB.

lobstr::obj_sizes(
    a = readGDX(gdx, 'v_demSeOth',  field = 'l', restore_zeros = FALSE),
    b = readGDX(gdx, 'v_demSeOth',  field = 'l', restore_zeros = TRUE))
# a:  3.35 kB
# b: 96.57 MB

image
@lecfab We had worse ;).

@orichters
Copy link
Contributor

Thanks for the explanation. The memory issue should be fixed anyway with #644

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member

I am aware. But this was as good a place as any to document how to figure out the memory use of restore_zeros = TRUE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants