Pyomo.DoE version 2 #2794

jialuw96 · 2023-04-03T03:03:58Z

Fixes

Reference #2610

Main fixes include:

Remove scenario index with block
Remove the flatenning functions, use a naming class for objects with more than 2 indexes
Deprecate sipopt
Use TicTocTimer
Replace string options with Enum
Documentation and tutorial enhancements
Add more tests

Summary/Motivation:

Update version 2 for doe. This version accomplishes enhancements in issue2610.

Tasks to complete before merging:

Review by @adowling2 on fork https://github.com/jialuw96/pyomo/tree/Feb_block_doe
Pass merge tests

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

I agree my contributions are submitted under the BSD license.
I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

…list.

jialuw96 · 2023-05-19T00:55:11Z

@blnicho Thank you for the reviews! I addressed them and saved a few of unsolved ones in #2610. The tests are passing now. Thank you!

codecov · 2023-05-19T21:15:37Z

Codecov Report

Patch coverage: 88.97% and project coverage change: +0.14 🎉

Comparison is base (2b51e1c) 87.17% compared to head (5c0405a) 87.32%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2794      +/-   ##
==========================================
+ Coverage   87.17%   87.32%   +0.14%     
==========================================
  Files         764      767       +3     
  Lines       88270    90240    +1970     
==========================================
+ Hits        76949    78801    +1852     
- Misses      11321    11439     +118

Flag	Coverage Δ
linux	`84.38% <88.97%> (+0.15%)`	⬆️
osx	`74.28% <40.42%> (+0.42%)`	⬆️
other	`84.56% <88.97%> (+0.15%)`	⬆️
win	`81.93% <88.97%> (+0.16%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pyomo/contrib/doe/scenario.py	`73.91% <70.00%> (-16.29%)`	⬇️
pyomo/contrib/doe/result.py	`31.72% <77.14%> (-2.75%)`	⬇️
pyomo/contrib/doe/doe.py	`83.13% <86.60%> (+15.84%)`	⬆️
pyomo/contrib/doe/measurements.py	`93.82% <93.82%> (+11.84%)`	⬆️
pyomo/contrib/doe/examples/reactor_compute_FIM.py	`95.00% <95.00%> (ø)`
pyomo/contrib/doe/examples/reactor_optimize_doe.py	`95.23% <95.23%> (ø)`
pyomo/contrib/doe/examples/reactor_grid_search.py	`95.83% <95.83%> (ø)`
pyomo/contrib/doe/examples/reactor_kinetics.py	`98.80% <98.80%> (ø)`
pyomo/contrib/doe/__init__.py	`100.00% <100.00%> (ø)`

... and 23 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

mrmundt · 2023-05-22T16:37:43Z

pyomo/contrib/doe/doe.py

-                        design_set_iter
-                    )[i]
+            for i, names in enumerate(design_dimension_names):
+                # names = design_dimension_names[i]


Leftover comment?

I removed this.

mrmundt · 2023-05-22T16:39:53Z

pyomo/contrib/doe/doe.py

-
-        Return:
+
+        Return
        -------
        m: the DOE model


Outdated: should be mod

But on that note, this is a nitpick: Is there an issue with just calling it model? Explicit variable names are a general preference.

I changed these to model.

mrmundt · 2023-05-22T16:43:43Z

pyomo/contrib/doe/doe.py

-            j: model responses
-            p: model parameters
-            t: timepoints
+            p: parameter


m is missing from docstring.

mrmundt · 2023-05-22T16:44:17Z

pyomo/contrib/doe/doe.py

            """
-            Calculate FIM elements
+            p: parameter


m missing from docstring.

mrmundt · 2023-05-22T16:47:27Z

pyomo/contrib/doe/examples/reactor_compute_FIM.py

Comment for all examples in general:

It would be nice to either have a README to explain the different examples or a small comment blurb in each example to explain what each example is showing (or, if the example is from a paper/book, a referencing to that paper/book).

mrmundt · 2023-05-22T16:50:31Z

pyomo/contrib/doe/measurements.py

+        for index_instance in all_variable_indices:
+            var_name_index_string = var_name + "["
+            for i, idx in enumerate(index_instance):
+                var_name_index_string += str(idx)


mrmundt · 2023-05-22T16:57:25Z

pyomo/contrib/doe/scenario.py

There is a lot going on in this file that is not tested. Highly recommend adding tests.

mod in doc string

adowling2

Added a comment

adowling2 · 2023-05-23T00:36:53Z

pyomo/contrib/doe/measurements.py

+        for index_instance in all_variable_indices:
+            var_name_index_string = var_name + "["
+            for i, idx in enumerate(index_instance):
+                var_name_index_string += str(idx)


This is on the todo list here: #2610

adowling2 · 2023-05-23T00:41:39Z

@jialuw96 I need you to merge this PR on your fork (jialuw96#16). I made the requested changes from @mrmundt but I do not have permission to write to your fork. Alternately, you can give me permission to write to your fork.

@mrmundt @blnicho I made the changes and opened a PR into @jialuw96's branch. The catch is that I did this on a computer that does not have a Pyomo development environment setup. I made the changes in the GitHub web browser. I checked them twice and I am relying on the tests to pass as an extra check. (These changes were to doc strings and renaming mod to model in one function which I triple-checked.)

@mrmundt The 👍 on your comments means I either addressed them in more fork or added them to our running todo list.

blnicho · 2023-05-23T02:17:15Z

@adowling2 I have push access and took care of merging your changes.

adowling2 · 2023-05-23T02:19:19Z

@blnicho. Awesome. I also got the tests passing on my fork, so fingers crossed, they should be here too.

jialuw96 · 2023-05-23T02:26:13Z

@adowling2 Thank you for taking care of this! I just sent an invitation for you to collaborate on this fork.

The requested changes have been made

jialuw96 added 30 commits February 7, 2023 15:27

doe changes

2864897

other changed files

401d4e0

examples updated

944f6fd

updates according to review

822543c

changed k_aug mode. Pending debugging

20687f6

K_aug debugged. Compute_FIM() can run with both modes now.

9a8d3e7

Update example model. Addressed reviews.

581a0e9

reformulate scenario class to fit block. debugged

1578cb8

delete unused meausrement class functions

0e7414b

changed measurement input style. used itertools to iterate all index …

bf8ea0a

…list.

well commented

6a35b1e

addressed part of comments. some changes pending discussion

b745622

generalize design variable fixing constraints

6067de5

Defining sepcialSet class. End of the day commit

ec498e7

test specialSet

87d102d

let measurement class inherite special set. debugged

55cb296

generalize design variables. debugged

b7bb080

EOD commit - generalizing _create_doe_model()

d42fdc4

generalize stochastic_program() function. begin debugging

9eccc9b

debugged optimization part

10a2ad1

commented

63af38b

update specialSet and measurements, and design variables

b502fe0

private function. debugged

a638086

addressed comments

14519ae

generalize run_grid search. going to generalize grid search result

adb90a2

grid search result debugged

db15231

comment run grid search

e5b6796

add variance

f4cb34e

variance debugged, commented

7fdfc22

design variable class added

9867abb

fix typo

d3cb944

blnicho and others added 3 commits May 19, 2023 13:45

NFC: Fixing Sphinx warnings and docstring formatting

b1ede4c

NFC: Fix black formatting

1aa4d81

NFC: Updates to doe.rst

8312ac3

blnicho added the AT: PRE-TEST INSPECTED label May 19, 2023

blnicho approved these changes May 19, 2023

View reviewed changes

pyomo-autotest removed the AT: PRE-TEST INSPECTED label May 19, 2023

mrmundt previously requested changes May 22, 2023

View reviewed changes

Removing unused comment

0986950

adowling2 mentioned this pull request May 23, 2023

Changes to address Pyomo.DoE edits jialuw96/pyomo#16

Merged

adowling2 added 2 commits May 22, 2023 20:25

Address model Pyomo.DoE review comments

e4f46f0

mod in doc string

Added missing m: Pyomo model from doc string

0262868

adowling2 reviewed May 23, 2023

View reviewed changes

adowling2 added 2 commits May 22, 2023 21:08

Fix black formatting error?

1f61bee

Fixed variable name typo

5c0405a

blnicho requested a review from mrmundt May 23, 2023 02:17

blnicho added the AT: PRE-TEST INSPECTED label May 23, 2023

pyomo-autotest removed the AT: PRE-TEST INSPECTED label May 23, 2023

blnicho merged commit e147a37 into Pyomo:main May 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pyomo.DoE version 2 #2794

Pyomo.DoE version 2 #2794

jialuw96 commented Apr 3, 2023 •

edited

Loading

jialuw96 commented May 19, 2023

codecov bot commented May 19, 2023 •

edited

Loading

mrmundt May 22, 2023

adowling2 May 23, 2023

mrmundt May 22, 2023

adowling2 May 23, 2023

mrmundt May 22, 2023

mrmundt May 22, 2023

mrmundt May 22, 2023

mrmundt May 22, 2023

mrmundt May 22, 2023

adowling2 left a comment

adowling2 May 23, 2023

adowling2 commented May 23, 2023

blnicho commented May 23, 2023

adowling2 commented May 23, 2023

jialuw96 commented May 23, 2023

Pyomo.DoE version 2 #2794

Pyomo.DoE version 2 #2794

Conversation

jialuw96 commented Apr 3, 2023 • edited Loading

Fixes

Summary/Motivation:

Tasks to complete before merging:

Legal Acknowledgement

jialuw96 commented May 19, 2023

codecov bot commented May 19, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adowling2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adowling2 commented May 23, 2023

blnicho commented May 23, 2023

adowling2 commented May 23, 2023

jialuw96 commented May 23, 2023

jialuw96 commented Apr 3, 2023 •

edited

Loading

codecov bot commented May 19, 2023 •

edited

Loading