You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 parameter relation_activity, which relates to the activity of technologies and must be defined at the sub-annual time level. The addition of time 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 and is_relation_lower are populated in the Java Backend side, which may make re-initializing these sets impossible or complex on the message_ix side, i.e., in models.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:
creating mirror sets and parameters for adding the missing index: For example, parameter relation_activity_time is introduced as the mirror of existing relation_activity, and similarly relation_cost_time, relation_lower_time and relation_upper_time are added.
creating mapping (or masking) sets on the fly in models.py. This includes is_relation_lower_time and is_relation_upper_time instead of existing is_relation_lower and is_relation_upper, respectively, which are produced on the Java backend.
adding duplicate blocks of equations (RELATION_EQUIVALENCE_TIME, RELATION_CONSTRAINT_UP_TIME, and RELATION_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
Check out this branch and run your existing scenarios, and they should work without a flaw.
Load model/scenario "MESSAGEix-CAS/relations_with_time_index" from "ene_ixmp" database and solve it using the code in this PR. This scenario does include the new items proposed in this PR. There shouldn't be any error when loading and solving that.
behnam-zakeri
changed the title
Extend user-defined relations to ncluding the index of time
Extend user-defined relations to include the index of timeDec 1, 2022
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.5%. Comparing base (d3cf5bd) to head (ec8a2bd). ⚠️ Report is 335 commits behind head on main.
#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 This sounds useful to me in principle :)
The new ixmp4 backend might make changes to how subannual time values are handled -- we can only respect the behavior that's covered in the current test suite. So it might be that things will soon change in a way that makes it harder for the PR to be merged. I think it would be easier for you to finalize this PR now, which shifts the burden of maintaining compatibility to ixmp4. If you don't have the time to do that yourself right now, you can consider raising it in a weekly meeting and asking for someone's time :)
This file contains hidden or 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
bugDoesn't work as advertised/unintended effectstimeslice
2 participants
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
timeindex.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 oftimeto 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_upperandis_relation_lowerare populated in the Java Backend side, which may make re-initializing these sets impossible or complex on themessage_ixside, 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_timeis introduced as the mirror of existingrelation_activity, and similarlyrelation_cost_time,relation_lower_timeandrelation_upper_timeare added.models.py. This includesis_relation_lower_timeandis_relation_upper_timeinstead of existingis_relation_lowerandis_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