Set period length parameters as indexed parameters by esrawli · Pull Request #76 · IDAES/idaes-gtep

esrawli · 2026-04-16T18:13:07Z

The periodLength parameters for each stage (representative period, commitment, and dispatch) are indexed by the number of periods on each respective stage. This will allow to determine different period lengths within each stage.

…mensionless (do not need to be multiplied by any time length)

These indexed parameters take now a list of values for each stage.

…called in the constraints

…and duration of periods while converting them to nested dictionaries for flexible period definitions In the model class, I included a function to expand scalars/lists to nested dictionaries for representative, commitment, and dispatch periods to support flexible period structures.

…tructure dictionary needed in the expansion model class

… duration from scalars or json file. Also, add test for this. In the GTEP class, remove function to expand scalars to a period structure dictionary and add this to a new function in utils file. Also, include new function that allows both, scalars (to be expanded to a period structure dictionary) or a .json file with customed period structure.

…exing-period-lengths

…the gtep class

…er and test to reflect these changes

… tests to avoid touching the error

…to avoid touching that error

…ncy error

… file to use in period structure test

…e consistency check

…ss and calculate them based on the provided scalars. Also, reorganize functions and update tests to reflect these changes.

jsiirola · 2026-05-18T17:16:03Z

-                    <= m.rampUpRates[generator]
-                    * b.dispatchPeriod[dispatchPeriod].periodLength
-                    * m.thermalCapacity[generator]


This looks like either the old implementation or the new implementation shouldn't be unit-consistent. Any idea which one is correct (and why the tests didn't catch it?)

jsiirola · 2026-05-18T17:32:38Z

                    <= max(
                        pyo.value(m.thermalMin[generator]),
-                        # [ESR: Make sure the time units are consistent
-                        # here since we are only taking the value]
                        pyo.value(m.rampUpRates[generator])
-                        * pyo.value(
-                            b.dispatchPeriod[dispatchPeriod].periodLength
-                        )  # in minutes
                        * pyo.value(m.thermalCapacity[generator]),
                    )


Design question: I generally don't like putting value() in model definitions. We could replace it with

<= MaxExpression(( m.thermalMin[generator], # [ESR: Make sure the time units are consistent # here since we are only taking the value] m.rampUpRates[generator] * b.dispatchPeriod[dispatchPeriod].periodLength) # in minutes * m.thermalCapacity[generator] ))

(Or if we know these are all NVP expressions (expressions of Params), then use a NPV_MaxExpression)

jsiirola · 2026-05-18T17:33:39Z

        num_commit=24,
        num_dispatch=1,
-        duration_dispatch=60,
+        duration_representative_period=24,


Should this br 24 * units.hour?

jsiirola · 2026-05-18T17:41:50Z

+import pyomo.environ as pyo
+from pyomo.environ import units as u
+
+curr_dir = os.path.dirname(os.path.abspath(__file__))


This can be a bit fragile. you might consider pyomo.common.fileutils.this_file_dir()

I would suggest renaming the variable, too: it is not holding the current (working) directory: it is holding this file's directory. Maybe:

Suggested change

curr_dir = os.path.dirname(os.path.abspath(__file__))

data_dir = os.path.join(pyomo.common.fileutils.this_file_dir(), 'data')

jsiirola · 2026-05-18T17:43:48Z

+            commitment_hr = duration_commitment[rep][com]
+            if abs(pyo.value(dispatch_sum_hr) - commitment_hr) > 1e-6:
+                dispatch_errors.append(
+                    f"  - Representative period {rep}, commitment period {com}: sum of dispatch durations ({pyo.value(dispatch_sum_hr)} hr) != commitment period duration ({commitment_hr} hr)"


Can you break this into multiple lines?

jsiirola · 2026-05-18T17:44:00Z

+        rep_period_hr = duration_representative_period[rep]
+        if abs(commitment_sum_hr - rep_period_hr) > 1e-6:
+            commitment_errors.append(
+                f"  - Representative period {rep}: sum of commitment durations ({commitment_sum_hr} hr) != representative period duration ({rep_period_hr} hr)"


Can you break this into multiple lines?

jsiirola · 2026-05-18T17:56:41Z

+        json_path = os.path.abspath(
+            os.path.join(curr_dir, "data", period_structure_json_file)
+        )


This implies that .json files are being distributed by IDAES-gtep. Can you confirm that they are being bundled in the wheels?

jsiirola · 2026-05-18T17:58:52Z

+
+        # Helper function to recursively convert string keys to
+        # integers
+        def convert_keys_to_int(obj):


This seems like a broadly useful capability. Should this be promoted to a general utility for "recovering" loaded general JSON files?

jsiirola · 2026-05-18T18:08:00Z

+        # under the data directory.
+        filename = (
+            os.path.abspath(
+                os.path.join(curr_dir, "data", "period_structure_from_gtep.json")


Again, is this file distributed in the wheel?

jsiirola · 2026-05-18T18:09:42Z

+    check_period_structure_consistency,
+)
+
+curr_dir = os.path.dirname(os.path.abspath(__file__))


This is fragile (and does not appear to be used). Remove?

jsiirola · 2026-05-18T18:10:35Z

+import pyomo.environ as pyo
+from pyomo.environ import units as u
+
+curr_dir = os.path.dirname(os.path.abspath(__file__))


I would suggest renaming the variable, too: it is not holding the current (working) directory: it is holding this file's directory. Maybe:

Suggested change

curr_dir = os.path.dirname(os.path.abspath(__file__))

data_dir = os.path.join(pyomo.common.fileutils.this_file_dir(), 'data')

jsiirola · 2026-05-18T18:17:35Z

+        json.dump(period_structure, f, indent=2)
+
+
+def generate_period_structure_utils(


I would recommend rethinking the name of this function (it doesn't generate "utils" does it?)

jsiirola · 2026-05-18T18:18:55Z

+    If a JSON file is specified, it loads the period structure from
+    that file.  Otherwise, generates the period structure from the
+    provided scalar arguments. Optionally saves the generated
+    structure to a JSON file.


This seems like three separate things. I would recommend these actions be three separate functions instead of switching them based on the arguments.

esrawli added 5 commits April 16, 2026 11:00

Remove period length from ramping constraints since ramp rates are di…

91f1223

…mensionless (do not need to be multiplied by any time length)

Replace periodLength parameters with new indexed parameters

22047dd

These indexed parameters take now a list of values for each stage.

Run black

e670b24

Rename PeriodLength parameters in each stage to avoid confusion when …

98b19d7

…called in the constraints

Update period length parameters names in test and run black

44cfd15

esrawli marked this pull request as draft April 16, 2026 19:42

esrawli and others added 4 commits April 16, 2026 16:35

Remove argument from class since it not needed anymore

c27a645

Remove len_reps since it is no longer needed or used

13ec972

Merge branch 'IDAES:main' into indexing-period-lengths

bfa512b

belle-storm mentioned this pull request Apr 23, 2026

gtep_data.py bug and hardcoding fix #61

Merged

esrawli added 7 commits April 29, 2026 14:33

Add new file "utils" that includes functions to generate the period s…

47b397a

…tructure dictionary needed in the expansion model class

Merge remote-tracking branch 'github-idaesgtep-esrawli/main' into ind…

cd0bcbf

…exing-period-lengths

Update test to call scalars from data file and not as an argument in …

e894155

…the gtep class

Move new arguments from expansion class to data class and update driv…

e7877a1

…er and test to reflect these changes

Add consistency check for the duration of dispatch periods and update…

0b0baf0

… tests to avoid touching the error

Add test for dispatch duration consistency check

570b10d

esrawli marked this pull request as ready for review April 29, 2026 23:49

esrawli requested review from blnicho and rundxdi April 29, 2026 23:49

esrawli added 6 commits April 30, 2026 08:54

Add consistency check for sum of commitment duration and update test …

1db3276

…to avoid touching that error

Add new duration for representative period to avoid touching consiste…

730ba25

…ncy error

Add header

7017099

Use Pyomo’s TempfileManager to create a temporary directory and .json…

2e6632f

… file to use in period structure test

Add functions that expand the period structure to a dictionary and th…

34a7a29

…e consistency check

remove duration for commitment and dispatch as an argument in the cla…

069625f

…ss and calculate them based on the provided scalars. Also, reorganize functions and update tests to reflect these changes.

jsiirola reviewed May 18, 2026

View reviewed changes

blnicho added the Priority:High High Priority Issue or PR label May 18, 2026

	curr_dir = os.path.dirname(os.path.abspath(__file__))
	data_dir = os.path.join(pyomo.common.fileutils.this_file_dir(), 'data')

		json.dump(period_structure, f, indent=2)


		def generate_period_structure_utils(

Conversation

esrawli commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

esrawli commented Apr 16, 2026 •

edited

Loading