pyomo
pyomo copied to clipboard
Add Pyomo.DOE to contrib
Fixes # .
Summary/Motivation: Contribute model-based design of experiments framework to Pyomo
Tasks to complete before merging:
- [ ] Merge PR on fork https://github.com/jialuw96/pyomo/pull/5
- [x] Add documentation in sphinx
- [ ] Confirm tests work
- [x] Add CCSI2 licence
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.
@jialuw96 @adowling2 what's the status on this WIP PR? Are you waiting on input from us?
As a general rule we tend to not focus any of our PR review resources on WIP PRs unless specifically asked.
Marked as ready for review to start working on finalizing the tests. @jsiirola @kaklise @adowling2 This is ready for first round of comments.
@jialuw96 I get an error when running the tutorial notebook. Cell 7 when calling compute_FIM
, "NameError: name 'if_Cholesky' is not defined"
@jialuw96 I get an error when running the tutorial notebook. Cell 7 when calling
compute_FIM
, "NameError: name 'if_Cholesky' is not defined"
Error solved. Now the tutorial notebook should be able to run without bug.
Tests are failing because idaes
is being imported unconditionally in test_reactor_example.py
. @jialuw96 last week you mentioned that you were importing idaes
in your testing example in order to get Ipopt. You should avoid importing idaes from files in the Pyomo repo because it could lead to issues with circular imports. You should assume the user would have Ipopt in their search path without needing to import idaes
.
Also, Pyomo does not have a strict dependency on Ipopt. This means that your testing examples using Ipopt should 1) verify that Ipopt is available and 2) skip the test when Ipopt is not available. You can find an example of this here: https://github.com/Pyomo/pyomo/blob/main/pyomo/dae/tests/test_initialization.py (see lines 26 and 102)
@blnicho Hi Bethany, we now have some errors from the code in docs. It seems that the code runs and print some sentences, and results, which is not expected by the doc. These sentences and results are printed by Pyomo.DOE by default. I attach two examples here. Should I prevent the code to print any sentences and not solve the problem in doctest? Thank you!
Here is an example where the example code prints sentences:
File "contributed_packages/doe/doe.rst", line 312, in default Failed example: measure_class = doe.Measurements(measure_pass, variance=measure_variance) # Use Pyomo.DoE.Measurements to achieve a measurement object Expected nothing Got: All measurements are flattened. Flatten measurement name: ['C_index_CA', 'C_index_CB', 'C_index_CC']
Here is an example where the example code solves problem and shows output:
File "contributed_packages/doe/doe.rst", line 336, in default Failed example: result = doe_object.compute_FIM(exp1,mode=sensi_opt, FIM_store_name = 'dynamic.csv', store_output = 'store_output') Expected nothing Got: This scenario: {'A1': {0: 84.87478999999999}, 'A2': {0: 371.72}, 'E1': {0: 7.78}, 'E2': {0: 15.05}, 'jac-index': {'A1': [0], 'A2': [0], 'E1': [0], 'E2': [0]}, 'eps-abs': {'A1': 0.08487478999999999, 'A2': 0.37172000000000005, 'E1': 0.0077800000000000005, 'E2': 0.015050000000000001}, 'scena-name': [0]} Ipopt 3.13.2: linear_solver=ma57 halt_on_ampl_error=yes max_iter=3000 <BLANKLINE> <BLANKLINE> ****************************************************************************** This program contains Ipopt, a library for large-scale nonlinear optimization. Ipopt is released as open source code under the Eclipse Public License (EPL). For more information visit http://projects.coin-or.org/Ipopt <BLANKLINE> This version of Ipopt was compiled from source code available at https://github.com/IDAES/Ipopt as part of the Institute for the De Advanced Energy Systems Process Systems Engineering Framework (IDAES PSE Framework) Copyright (c) 2018-2019. See https://github.com/IDAES/idaes-pse. <BLANKLINE> This version of Ipopt was compiled using HSL, a collection of Fortran codes for large-scale scientific computation. All technical papers, sales and publicity material resulting from use of the HSL codes within IPOPT must contain the following acknowledgement: HSL, a collection of Fortran codes for large-scale scientific computation. See http://www.hsl.rl.ac.uk./ ****************************************************************************** <BLANKLINE> This is Ipopt version 3.13.2, running with linear solver ma57. <BLANKLINE> Number of nonzeros in equality constraint Jacobian...: 2955
(and then there's the IPOPT output)
@jialuw96 you can run the documentation tests locally by going into the pyomo/doc/OnlineDocs
directory and running the command make doctest -d
For examples of writing doctests that include output take a look at these rst files: https://github.com/Pyomo/pyomo/blob/main/doc/OnlineDocs/contributed_packages/trustregion.rst https://github.com/Pyomo/pyomo/blob/main/doc/OnlineDocs/contributed_packages/sensitivity_toolbox.rst
In particular, if your code snippets produce output (like the print statements at the end of your doe.rst
file) then you need to include that output in the code snippet. The doctest will verify that the code runs and the same output is produced. If an exact comparison doesn't make sense or is too fragile (i.e. full Ipopt log or floating point number with many decimal places) then there are additional options you can specify in the doctest (...
or :skipif:
or doctest: +SKIP
) or you can be more careful in what is printed (rounding floats to fixed number of decimal places).
@blnicho Hi Bethany, I included the output of print sentences, and skip commands that would generate IPOPT outputs. The error now is a MPI error in linux/3.8/mpi as:
Run # Manually invoke the DAT parser so that parse_table_datacmds.py /home/runner/work/_temp/56e3fe61-c523-4458-8cf3-31fce554ad5f.sh: line 5: mpirun: command not found Error: Process completed with exit code 127.
and codev error in proces-coverage:
[2022-06-21T17:13:47.903Z] ['error'] There was an error running the uploader: Error uploading to https://codecov.io: Error: There was an error fetching the storage URL during POST: 404 - {'detail': ErrorDetail(string='Could not find a repository, try using repo upload token', code='not_found')} Error: Codecov: Failed to properly upload: The process '/home/runner/work/_actions/codecov/codecov-action/v2/dist/codecov' failed with exit code 255
Could you point out how to debug these errors from my side? Thank you!
Kanishka is also having codecov issues. Perhaps discuss this at the Pyomo dev call today?
Codecov Report
Base: 87.04% // Head: 86.68% // Decreases project coverage by -0.37%
:warning:
Coverage data is based on head (
4d504f1
) compared to base (09e204c
). Patch coverage: 66.05% of modified lines in pull request are covered.
Additional details and impacted files
@@ Coverage Diff @@
## main #2294 +/- ##
==========================================
- Coverage 87.04% 86.68% -0.37%
==========================================
Files 757 766 +9
Lines 84596 86224 +1628
==========================================
+ Hits 73638 74742 +1104
- Misses 10958 11482 +524
Flag | Coverage Δ | |
---|---|---|
linux | 84.15% <66.05%> (-0.30%) |
:arrow_down: |
osx | 73.59% <17.41%> (-0.92%) |
:arrow_down: |
other | 84.34% <66.05%> (-0.30%) |
:arrow_down: |
win | 81.54% <66.05%> (-0.26%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
pyomo/contrib/doe/result.py | 34.27% <34.27%> (ø) |
|
pyomo/contrib/sensitivity_toolbox/sens.py | 97.40% <50.00%> (-0.31%) |
:arrow_down: |
pyomo/contrib/doe/doe.py | 67.28% <67.28%> (ø) |
|
pyomo/contrib/doe/measurements.py | 81.98% <81.98%> (ø) |
|
pyomo/contrib/doe/scenario.py | 90.19% <90.19%> (ø) |
|
pyomo/contrib/doe/example/reactor_grid_search.py | 93.47% <93.47%> (ø) |
|
pyomo/contrib/doe/example/reactor_compute_FIM.py | 94.44% <94.44%> (ø) |
|
pyomo/contrib/doe/example/reactor_optimize_doe.py | 96.15% <96.15%> (ø) |
|
pyomo/contrib/doe/example/reactor_kinetics.py | 97.93% <97.93%> (ø) |
|
pyomo/contrib/doe/__init__.py | 100.00% <100.00%> (ø) |
|
... and 5 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
@blnicho Hi Bethany, the comments on doe.rst are addressed now and unused figures are removed. Thank you!
@blnicho The comments are addressed. Currently no other questions. Thank you!
@jialuw96 I just pushed a minor change to your notebook example. This just removes the cell ids and tells Jupyter to use a slightly older notebook format.
Hi Bethany, thank you for comments! We are still working on them. I would like to follow up about changing the tutorial notebook into a test:
- What is preferred is we make the examples a .py file, create a .py file for each function I call Pyomo.DOE, and add a main function to each .py file. In the test folder, I then add a .py file where I call every .py file for this notebook as a test. Right?
- We can still keep the tutorial notebook, right?
Thank you!
@blnicho Hi Bethany, sorry I cannot attend the meeting today due to a meeting conflict. I have modified this version and it passes all tests in my local machine; However it shows one error message I never saw in the workflow, it is pasted below:
/Users/runner/work/pyomo/pyomo/pyomo/environ/tests/test_package_layout.py:72: AssertionError ------ generated xml file: /Users/runner/work/pyomo/pyomo/TEST-pyomo.xml ------- =========================== short test summary info ============================ FAILED pyomo/core/tests/unit/test_symbolic.py::SymbolicDerivatives::test_trig_fuctions FAILED pyomo/environ/tests/test_package_layout.py::TestPackageLayout::test_for_init_files ===== 2 failed, 9690 passed, 5016 skipped, 11 xfailed in 900.13s (0:15:00) ===== Error: Process completed with exit code 1.
Do you have idea how to fix this? Thank you!
Jialu
@jialuw96 I'm not seeing the same test failures you reported. Looks like most of the failing jobs are all failing because Ipopt is failing to converge. This is the test that needs to be fixed:
pyomo/contrib/doe/tests/test_example.py::TestReactorExample::test_reactor_optimize_doe
The PyPy job also revealed a couple tests that require optional dependencies (pandas or scipy):
FAILED pyomo/contrib/doe/tests/test_example.py::TestReactorExample::test_reactor_compute_FIM
FAILED pyomo/contrib/doe/tests/test_example.py::TestReactorExample::test_reactor_grid_search
You'll need to check if these dependencies are available and skip the tests when they aren't installed (similar to how you're checking ipopt_available
).
@blnicho Hi Bethany, I think all tests get passed now. I would like to confirm on what are needed for the 3rd round of reviews:
- [x] Keep copyright info of Pyomo and CCSI2 in every file, while moving CCSI2 license to be a separate .txt file.
- [x] Use logging instead of print statements
- [x] Remove build directory
Is there anything else that you are expecting? I plan to finish these and make this repo ready for next round of review this and the next week. Thank you!
@jialuw96 I added a couple more items to the checklist in your last comment.
@adowling2 @blnicho @jsiirola This code is ready for the next round of reviews. Thank you!
@jialuw96 As we work through these comments, we need to document any enchancements we decide to delay to a future PR on the new meta issue #2610 .
@blnicho @jsiirola Just to confirm, our intention is to merge this after the November release. We then have a few smaller PRs planned for January and early February (see #2610) to get merged before the next Pyomo release.
@blnicho @jsiirola @adowling2 This pull request is ready for review. Thank you!
@blnicho @jsiirola @adowling2 This PR is ready for review. Thank you!
@jialuw96 You replied to my comments but I think you forgot to push the updates.
@kaklise @blnicho @jsiirola I think this is ready for one of you to complete the 2nd review.
@blnicho This PR is ready for review. Thank you!
@jialuw96 I'm still seeing errors in the ReadtheDocs build with some of the autoclass commands. This is the output:
WARNING: autodoc: failed to import class 'fim_doe.DesignOfExperiments' from module 'pyomo.contrib.doe'; the following exception was raised:
No module named 'pyomo.contrib.doe.fim_doe'
/home/docs/checkouts/readthedocs.org/user_builds/pyomo/checkouts/2294/pyomo/contrib/doe/measurements.py:docstring of pyomo.contrib.doe.measurements.Measurements.__init__:4: WARNING: Definition list ends without a blank line; unexpected unindent.
WARNING: autodoc: failed to import class 'FIM_result' from module 'pyomo.contrib.doe'; the following exception was raised:
Traceback (most recent call last):
File "/home/docs/checkouts/readthedocs.org/user_builds/pyomo/envs/2294/lib/python3.7/site-packages/sphinx/util/inspect.py", line 376, in safe_getattr
return getattr(obj, name, *defargs)
AttributeError: module 'pyomo.contrib.doe' has no attribute 'FIM_result'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/docs/checkouts/readthedocs.org/user_builds/pyomo/envs/2294/lib/python3.7/site-packages/sphinx/ext/autodoc/importer.py", line 98, in import_object
obj = attrgetter(obj, mangled_name)
File "/home/docs/checkouts/readthedocs.org/user_builds/pyomo/envs/2294/lib/python3.7/site-packages/sphinx/ext/autodoc/__init__.py", line 306, in get_attr
return autodoc_attrgetter(self.env.app, obj, name, *defargs)
File "/home/docs/checkouts/readthedocs.org/user_builds/pyomo/envs/2294/lib/python3.7/site-packages/sphinx/ext/autodoc/__init__.py", line 2804, in autodoc_attrgetter
return safe_getattr(obj, name, *defargs)
File "/home/docs/checkouts/readthedocs.org/user_builds/pyomo/envs/2294/lib/python3.7/site-packages/sphinx/util/inspect.py", line 392, in safe_getattr
raise AttributeError(name) from exc
AttributeError: FIM_result
WARNING: autodoc: failed to import class 'Grid_Search_Result' from module 'pyomo.contrib.doe'; the following exception was raised:
Traceback (most recent call last):
File "/home/docs/checkouts/readthedocs.org/user_builds/pyomo/envs/2294/lib/python3.7/site-packages/sphinx/util/inspect.py", line 376, in safe_getattr
return getattr(obj, name, *defargs)
AttributeError: module 'pyomo.contrib.doe' has no attribute 'Grid_Search_Result'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/docs/checkouts/readthedocs.org/user_builds/pyomo/envs/2294/lib/python3.7/site-packages/sphinx/ext/autodoc/importer.py", line 98, in import_object
obj = attrgetter(obj, mangled_name)
File "/home/docs/checkouts/readthedocs.org/user_builds/pyomo/envs/2294/lib/python3.7/site-packages/sphinx/ext/autodoc/__init__.py", line 306, in get_attr
return autodoc_attrgetter(self.env.app, obj, name, *defargs)
File "/home/docs/checkouts/readthedocs.org/user_builds/pyomo/envs/2294/lib/python3.7/site-packages/sphinx/ext/autodoc/__init__.py", line 2804, in autodoc_attrgetter
return safe_getattr(obj, name, *defargs)
File "/home/docs/checkouts/readthedocs.org/user_builds/pyomo/envs/2294/lib/python3.7/site-packages/sphinx/util/inspect.py", line 392, in safe_getattr
raise AttributeError(name) from exc
AttributeError: Grid_Search_Result