-
Notifications
You must be signed in to change notification settings - Fork 154
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
Extend user-defined relations to include the index of time
#680
Draft
behnam-zakeri
wants to merge
13
commits into
iiasa:main
Choose a base branch
from
behnam-zakeri:relation-time
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+245
−35
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
behnam-zakeri
changed the title
Extend user-defined relations to ncluding the index of
Extend user-defined relations to include the index of Dec 1, 2022
time
time
behnam-zakeri
force-pushed
the
relation-time
branch
from
December 2, 2022 13:35
0435076
to
6691ee1
Compare
behnam-zakeri
force-pushed
the
relation-time
branch
from
March 10, 2023 10:45
518a514
to
b8b628d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #680 +/- ##
=====================================
Coverage 95.5% 95.5%
=====================================
Files 46 46
Lines 4340 4351 +11
=====================================
+ Hits 4148 4159 +11
Misses 192 192
|
#705 already includes the content of commit 08ef0c7, so when you continue working on this PR, you can/have to exclude this commit. See this comment for how to do that. |
behnam-zakeri
force-pushed
the
relation-time
branch
from
July 4, 2023 08:40
b8b628d
to
63a4b0c
Compare
7 tasks
behnam-zakeri
force-pushed
the
relation-time
branch
from
October 17, 2024 10:35
63a4b0c
to
e509494
Compare
behnam-zakeri
force-pushed
the
relation-time
branch
from
October 17, 2024 11:03
80ea481
to
ec8a2bd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR re-introduces user-defined relations (sets, parameters, equations, variables) with the inclusion of sub-annual
time
index.Background: The user-defined relations in the GAMS formulation do not represent index of
time
, which can be an issue when working on models with sub-annual time slices (see #191). For example, it is not possible to represent time slices in parameterrelation_activity
, which relates to the activity of technologies and must be defined at the sub-annual time level. The addition oftime
to existing sets and parameters means re-initializing these items, which will create backward incompatibility. Via #631 a workaround was suggested, which was implemented in PR#633. The improvements proposed in this PR could use the same workaround as #633, however, a few sets here, namely,is_relation_upper
andis_relation_lower
are populated in the Java Backend side, which may make re-initializing these sets impossible or complex on themessage_ix
side, i.e., inmodels.py
.Solution: Until a robust solution will be in place for re-initializing sets and parameters that are at the moment being populated at the Java side in
message_ix
, this PR proposes an interim solution as follows:relation_activity_time
is introduced as the mirror of existingrelation_activity
, and similarlyrelation_cost_time
,relation_lower_time
andrelation_upper_time
are added.models.py
. This includesis_relation_lower_time
andis_relation_upper_time
instead of existingis_relation_lower
andis_relation_upper
, respectively, which are produced on the Java backend.RELATION_EQUIVALENCE_TIME
,RELATION_CONSTRAINT_UP_TIME
, andRELATION_CONSTRAINT_LO_TIME
) for enhancing the mathematical formulation.This PR remains as draft for the time being. Merging this PR in the main branch shouldn't create any problems for running existing scenarios. However, having duplicate items (set, parameter, variable and equations) proposed in this PR may not be an optimal solution from a design perspective.
How to review
PR checklist