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

Add offshore nodes to YAML #3

Merged
merged 21 commits into from
Dec 18, 2024
Merged

Conversation

JKNTNU
Copy link
Contributor

@JKNTNU JKNTNU commented Dec 13, 2024

Hi Daniel,

As discussed, I added the offshore nodes to the YAML. I hope the way I did it is correct.

Best,

Jannis

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

See a first round of comments inline.

mappings/empire_v2.0.yaml Outdated Show resolved Hide resolved
mappings/empire_v2.0.yaml Outdated Show resolved Hide resolved
mappings/empire_v2.0.yaml Outdated Show resolved Hide resolved
mappings/empire_v2.0.yaml Outdated Show resolved Hide resolved
mappings/empire_v2.0.yaml Outdated Show resolved Hide resolved
mappings/empire_v2.0.yaml Outdated Show resolved Hide resolved
mappings/empire_v2.0.yaml Outdated Show resolved Hide resolved
mappings/empire_v2.0.yaml Outdated Show resolved Hide resolved
mappings/empire_v2.0.yaml Outdated Show resolved Hide resolved
mappings/empire_v2.0.yaml Outdated Show resolved Hide resolved
@JKNTNU
Copy link
Contributor Author

JKNTNU commented Dec 13, 2024

In the results data, it is called "The Netherlands", should it be like this here as well then? Or does that not matter?

@danielhuppmann
Copy link
Member

In the results data, it is called "The Netherlands", should it be like this here as well then? Or does that not matter?

For consistency across the project, the region in the database has to be in line with the spelling in the european-coountries.yaml.

There are two options to get that result:

  1. you use the renaming-feature in the mapping file like
    The Netherlands: Netherlands
  2. or you directly change it in your output processing.

Given that you will also want to report directional results, it may be easier to use the second approach.

@JKNTNU
Copy link
Contributor Author

JKNTNU commented Dec 16, 2024

Okay! Thank you! I will change it in the output file.

@JKNTNU JKNTNU closed this Dec 16, 2024
@JKNTNU JKNTNU reopened this Dec 16, 2024
@JKNTNU
Copy link
Contributor Author

JKNTNU commented Dec 16, 2024

Hi Daniel,

I implemented the changes now and hope that everything is correct. Do I open a new PR then or can I renew this one?

@danielhuppmann
Copy link
Member

No need to open a new PR, just committing to a branch with an open PR and pushing to Github is sufficient.

@JKNTNU
Copy link
Contributor Author

JKNTNU commented Dec 16, 2024

Okay. That should be done if I am not mistaken.

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Thanks!

Two questions:

  1. Do you want to have region-processing from country-level to European aggregates? Or are you going to include all relevant aggregates in your scenario-submission files? If you want to use region-aggregation processing as part of the submission to the Scenario Explorer database (and have smaller files for the upload), you should add a mapping file.
  2. Are there other disaggregations of the North Sea areas? Or will all models in iDesignRES that have such level of detail use the same regional disaggregation. Your specification of territories of "North Sea" in the file european-regions.yaml implies that this is the one and only disaggregation to be used. If that is indeed the case, it would make sense to define the different subdivisions of the North Sea there, and not make them specific to EMPIRE.

Also, please remove the file connections.yaml for now. As a first test upload of a scenario, having the countries and subdivisions will be sufficient. Once you have successfully uploaded such a file, we can add scenario results on interconnections.

Also, we will work on additional validation features so that items like "NO1>NO2" are not allowed (I assume that these should be renamed to proper North Sea areas).

definitions/region/connections.yaml Outdated Show resolved Hide resolved
@JKNTNU
Copy link
Contributor Author

JKNTNU commented Dec 17, 2024

Hi Daniel,

Thanks for your reply. I have to double check this and discuss it with Mostafa. My task is to upload to EMPIRE-data from the run I did but I am not sure about the project-wide details regarding these questions, so I don't want to give a wrong answer here. I'll get back to you once I know more.

Another question:
I am not sure right now, but did I already register this EMPIRE run as a model for the scenario explorer? Else I'd do it soon as well so that I can upload the data as fast as possible.

Best,

Jannis

@JKNTNU
Copy link
Contributor Author

JKNTNU commented Dec 17, 2024

Hi Daniel,

I will check for the aggregations in the data now.
Regarding the questions:
We’re uncertain which nodes the other models will consider as the North Sea region. Do you have any recommendations for this?
Regarding NO1 and NO2 (Norway's sub-regions), could you clarify why they are not allowed?
Thanks in advance!

definitions/region/european-regions.yaml Outdated Show resolved Hide resolved
definitions/region/empire_v2.0.yaml Outdated Show resolved Hide resolved
definitions/region/connections.yaml Outdated Show resolved Hide resolved
@danielhuppmann
Copy link
Member

did I already register this EMPIRE run as a model for the scenario explorer?

The discussion we are having here is the model registration. Once we have agreed on the region-naming and merged this PR, you can submit scenarios.

why are NO1 and NO2 not allowed?

The region-names are "EMPIRE v2|Norway|Ostland", etc. - not NO1. Please go through that list carefully and double-check that you are using the correct region names.

We’re uncertain which nodes the other models will consider as the North Sea region. Do you have any recommendations for this?

Let's keep this as "EMPIRE v2|North Sea|..." for now.

@JKNTNU
Copy link
Contributor Author

JKNTNU commented Dec 17, 2024

Hi Daniel,

Sorry for having to annoy you with this so much. I was asked to rename the model run back to EMPIRE1.0 and then only change the scenario for the name. Is that possible?

Best,

Jannis

@danielhuppmann
Copy link
Member

I was asked to rename the model run back to EMPIRE1.0 and then only change the scenario for the name. Is that possible?

Yes, this is possible, but I advise to use different version numbers when the regional resolution of a model changes. For example, this would make it very confusing when using the region-processing of the Scenario Explorer infrastructure.

@JKNTNU
Copy link
Contributor Author

JKNTNU commented Dec 17, 2024

Mostafa told me to ask if we could go for model name: V1.0.0/v51 . Is that fine?

@danielhuppmann
Copy link
Member

Mostafa told me to ask if we could go for model name: V1.0.0/v51 . Is that fine?

Why?

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Sorry for being a pain but the region-definitions have to be correct...

definitions/region/connections.yaml Show resolved Hide resolved
definitions/region/connections.yaml Outdated Show resolved Hide resolved
definitions/region/connections.yaml Outdated Show resolved Hide resolved
definitions/region/connections.yaml Outdated Show resolved Hide resolved
@JKNTNU
Copy link
Contributor Author

JKNTNU commented Dec 18, 2024

Regarding the model name/version:
The version of the EMPIRE we used is V 1.0.0 and we want to keep the consistency. V51 is the version for the European data, therefore this specification.

@danielhuppmann
Copy link
Member

The version of the EMPIRE we used is V 1.0.0 and we want to keep the consistency. V51 is the version for the European data, therefore this specification.

Ok, then please rename all regions in this PR from "EMPIRE v2" to "EMPIRE V1.0.0/v51" if that's what you want.

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Sorry...

@@ -0,0 +1,193 @@
- Connections:
# allow directional connections explicitly
- Austria>Czechia
Copy link
Member

Choose a reason for hiding this comment

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

The (directional) region names following the hierarchy have to be indented, like in the example shown in the other thread.

Suggested change
- Austria>Czechia
- Austria>Czechia

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Thank you, tests are passing!

@danielhuppmann danielhuppmann merged commit 7269169 into iiasa:main Dec 18, 2024
2 checks passed
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.

2 participants