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

Delete non-ldv entries (extensive) #587

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

jmuessel
Copy link
Contributor

@jmuessel jmuessel commented Apr 15, 2024

In the new version, using reporttransport, non-ldv variables will not be reported anymore. With this PR

  1. non-ldv variables that are related to COMPLEX will be deleted.
  2. For this to function the required minimal version of connected packages is stated.

In another PR, I already renamed variables and deleted non-LDV variables from rmd skripts but not from the other R skripts as I am not sure about the consequences - see discussion below.

ToDos

  • build package
  • check that in other packages the correct remind2 version is required (PIAM interfaces e.g.)

@jmuessel jmuessel requested a review from fbenke-pik June 6, 2024 14:20
@@ -342,7 +342,6 @@ reportPrices <- function(gdx, output=NULL, regionSubsetList=NULL,
fe = c(
fehos = "Liquids",
fepet = "LDV|Liquids",
Copy link
Contributor

@fbenke-pik fbenke-pik Jun 7, 2024

Choose a reason for hiding this comment

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

@orichters Due to deletion of 'fedie' , the function addVar throws the warning `In reportPrices, addVar called with a NA value, e.g. "Price|Final Energy|Transport|other|NA|Fossil (US$2005/GJ)".

In consequence, duplicates are produced in the out object, and that is a problem.

What would be the correct way to deal with this? Not delete the fedie entries or somehow address them being removed?

Copy link
Contributor

@fbenke-pik fbenke-pik Jun 7, 2024

Choose a reason for hiding this comment

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

@orichters Btw, I think a warning in in addVar is not sufficient when there are NAs. Should crash or return NULL, as the returned magclass object becomes unusable, yet it is not easy to find the source of the problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please consult with @Renato-Rodrigues who introduced the addVar function. I don't understand how this can create duplicates, though: this line should avoid it.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right, the duplicates are produced in another part of the code.

We still have to avoid the warning, @jmuessel as it will be thrown 50+ times, i.e. individual warnings are no longer printed and do not appear in the log.

@jmuessel jmuessel changed the title Delete non-ldv entries Delete non-ldv entries (extensive) Jul 2, 2024
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.

3 participants