Updated Pyomo.DoE tutorial notebook. #2889

hglynch · 2023-06-27T13:53:12Z

Fixes # .

Summary/Motivation:

Pyomo.DoE tutorial notebooks would benefit from better explanations for users new to model-based design of experiments

Changes proposed in this PR:

Added optional code to run on Google Colab (and install solvers via IDAES)
Improved explanations
Added recommended reading

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.

jsiirola · 2023-06-27T14:45:02Z

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

+    "import sys\n",
+    "# If running on Google Colab, install Ipopt via IDAES\n",
+    "if \"google.colab\" in sys.modules:\n",
+    "    !wget \"https://raw.githubusercontent.com/ndcbe/CBE60499/main/notebooks/helper.py\"\n",


Is this the best place to distribute helper.py? Maybe it should be incorporated into the idaes-pse repo? @adowling2?

I'm considering updating the notebook to just give the helper code here.

Another option would be to have a separate package released by ideas, say colab-pyomo-helper. This would then make it !pip install colab-pyomo-helper.

I just looked at the helper code, and it is sufficiently complex (and not directly tied to DoE) that I would not embed it here. I also think it is way too much overhead to have (yet another) package that has to be released. Plus, there is little / no value to being able to install previous versions of it.

It seems pretty simple to put it in idaes/scripts and download it here. That has the side benefit that everyone can reference the same script, so as things evolve, we can keep just one script up-to-date.

@hglynch Change code to as follows:

if "google.colab" in sys.modules: !wget "https://raw.githubusercontent.com/adowling2/idaes-pse/tree/colab-install-script/scripts/colab_helper.py" import colab_helper colab_helper.install_idaes() colab_helper.install_ipopt()

Does the IDAES and Ipopt install still work on Colab?

@hglynch you should change the intro code to:

if "google.colab" in sys.modules: !wget "https://raw.githubusercontent.com/idaes-pse/main/scripts" import colab_helper colab_helper.install_idaes() colab_helper.install_ipopt()

adowling2 · 2023-06-27T15:28:56Z

I'll add it there. Thank you in advance for reviewing ;)

hglynch · 2023-08-06T05:12:57Z

@adowling2 completed updates for Pyomo.DoE tutorial, ready for review

blnicho · 2023-08-08T22:47:14Z

@hglynch @adowling2 the August Pyomo release is currently scheduled for August 23rd. Is this PR something you want included in the release?

adowling2

@hglynch Here are some minor edits. Once these are done, retag myself and John.

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

adowling2 · 2023-08-09T12:14:17Z

@blnicho Thank you for the nudge. Yes, I'd like to include this in the release. These changes make the Pyomo.DoE tutorial more accessible. @hglynch, Can you remove the [WIP] from the name of this PR after making the edits I requested? That tells the Pyomo team this is ready to review.

mrmundt · 2023-08-15T18:44:26Z

@hglynch , @adowling2 - Reminder that the target release for Pyomo is next week. If you'd like to get this in before the release, please make edits before this weekend, if possible.

adowling2 · 2023-08-15T19:45:06Z

Another barrier is getting @jsiirola to review and approve this PR: IDAES/idaes-pse#1237

adowling2 · 2023-08-18T16:55:56Z

@hglynch here are the action items, which should be quick:

Address my comments and retest the notebook on Google Colab. We want to check the new installation script that is now available via the IDAES git repo. (See my comment above for specific instructions.)
Remove [WIP] from this PR name
Request reviews from @jsiirola @blnicho @mrmundt and myself. You can also post a comment here and tag us to make sure we get notified.

hglynch · 2023-08-19T03:48:25Z

@adowling2 @jsiirola @blnicho @mrmundt This PR is ready for final review.

adowling2

Looks great.

adowling2 · 2023-08-21T13:43:18Z

@hglynch One minor change:

!wget "https://raw.githubusercontent.com/idaes-pse/main/scripts"

Should become

!wget "https://raw.githubusercontent.com/idaes-pse/main/scripts/colab_helper.py"

Please also verify this notebook runs on Colab.

hglynch · 2023-08-21T14:27:04Z

@adowling2 I am receiving an error when I try running it in colab: "no module named colab_helper"

adowling2 · 2023-08-21T14:54:51Z

@hglynch Did you successfully run this line first?

!wget "https://raw.githubusercontent.com/idaes-pse/main/scripts/colab_helper.py"

That downloads colab_helper.py.

hglynch · 2023-08-21T15:05:36Z

@adowling2 Yes I ran that first, here is the error:

adowling2 · 2023-08-21T15:14:00Z

Ah, you can see the 404 error in the output. That means the script was not being downloaded correctly. Instead, try:

!wget "https://raw.githubusercontent.com/IDAES/idaes-pse/main/scripts/colab_helper.py"

hglynch · 2023-08-21T16:07:07Z

@adowling2 It works in colab now, I adjusted it in the jupyter notebook as well.

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

mrmundt · 2023-08-21T16:36:17Z

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "## Key Takeaways\n",


I like the key takeaways. Can you maybe put them at the top and the bottom? That way I know what I am going to learn upfront and am reminded at the end what you taught me.

mrmundt · 2023-08-21T17:23:17Z

@hglynch - Please check out the Contributing guide for what tools we use for the automatic linting.

hglynch · 2023-08-22T16:49:38Z

@hglynch - Please check out the Contributing guide for what tools we use for the automatic linting.

@mrmundt I looked through this page and searched the documentation for "lint" and did not find anything. Can you provide a link to an example or give the commands to run for the automatic linting?

mrmundt · 2023-08-22T16:52:22Z

@hglynch - Please check out the Contributing guide for what tools we use for the automatic linting.

@mrmundt I looked through this page and searched the documentation for "lint" and did not find anything. Can you provide a link to an example or give the commands to run for the automatic linting?

See here: https://pyomo.readthedocs.io/en/stable/contribution_guide.html#coding-standards - black and spell-checker

hglynch · 2023-08-22T22:25:39Z

@hglynch - There are still a few typos that need fixed, according to our automatic checker. Sorry for the struggle!

@mrmundt Thank you! I fixed the typos that I could find, but I am unable to install the spell checker so if you are able to let me know if there are anymore I can fix them!

mrmundt

Thanks for addressing it all! Looks great.

codecov · 2023-08-23T04:09:39Z

Codecov Report

Patch coverage has no change and project coverage change: -0.01% ⚠️

Comparison is base (c569b78) 87.84% compared to head (40c32d2) 87.83%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2889      +/-   ##
==========================================
- Coverage   87.84%   87.83%   -0.01%     
==========================================
  Files         770      770              
  Lines       89665    89665              
==========================================
- Hits        78764    78756       -8     
- Misses      10901    10909       +8

Flag	Coverage Δ
linux	`84.87% <ø> (ø)`
osx	`74.58% <ø> (ø)`
other	`85.04% <ø> (ø)`
win	`82.11% <ø> (-0.02%)`	⬇️

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

see 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

blnicho · 2023-08-23T06:20:45Z

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

  {
   "cell_type": "code",
   "execution_count": 2,
-   "metadata": {},
+   "metadata": {
+    "tags": []
+   },
+   "outputs": [
+    {
+     "name": "stdout",
+     "output_type": "stream",
+     "text": [
+      "Successfully loaded idaes.\n"
+     ]
+    }
+   ],
+   "source": [
+    "# Ipopt installer\n",
+    "import sys\n",
+    "\n",
+    "# If running on Google Colab, install Ipopt via IDAES\n",
+    "if \"google.colab\" in sys.modules:\n",
+    "    !wget \"https://raw.githubusercontent.com/IDAES/idaes-pse/main/scripts/colab_helper.py\"\n",
+    "    import colab_helper\n",
+    "\n",
+    "    colab_helper.install_idaes()\n",
+    "    colab_helper.install_ipopt()\n",
+    "\n",
+    "# Otherwise, attempt to load IDAES which should include Ipopt and k_aug\n",
+    "# See https://idaes-pse.readthedocs.io/en/stable/tutorials/getting_started/index.html\n",
+    "# for instructions on running IDAES get-extensions\n",
+    "else:\n",
+    "    try:\n",
+    "        import idaes\n",
+    "\n",
+    "        # Provided IDAES extensions are installed, importing IDAES provides access to\n",
+    "        # Ipopt with HSL and k_aug which are needed for this example\n",
+    "        print(\"Successfully loaded IDAES.\")\n",
+    "    except:\n",
+    "        print(\n",
+    "            \"IDAES is not installed. Make sure you have independently installed Ipopt with HSL and k_aug.\"\n",
+    "        )"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": 3,
+   "metadata": {
+    "tags": []
+   },
   "outputs": [],
   "source": [
-    "## check if ipopt available \n",
-    "ipopt_available = pyo.SolverFactory('ipopt').available()\n",
+    "# Check if Ipopt is available\n",
+    "ipopt_available = pyo.SolverFactory(\"ipopt\").available()\n",
    "if not (ipopt_available):\n",
-    "    raise RuntimeError('This Pyomo.DoE example requires Ipopt.')"
+    "    raise RuntimeError(\"This Pyomo.DoE example requires Ipopt.\")"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": 1,
+   "metadata": {
+    "tags": []
+   },
+   "outputs": [],
+   "source": [
+    "# Imports\n",
+    "import numpy as np\n",
+    "import pyomo.environ as pyo\n",
+    "from pyomo.dae import ContinuousSet, DerivativeVar\n",
+    "from pyomo.contrib.doe import (\n",
+    "    ModelOptionLib,\n",
+    "    DesignOfExperiments,\n",
+    "    MeasurementVariables,\n",
+    "    DesignVariables,\n",
+    ")"
   ]
  },


These first three setup cells are out of order. The output captured in the notebook notes that they were executed in a different order than they appear in order to run successfully. In particular, the second cell checking solver availability uses the pyo namespace which isn't imported until the third cell. Could you fix the ordering and rerun the notebook such that the cells are run in order?

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

blnicho · 2023-08-23T06:50:47Z

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

+      "  22 -1.4309111e+01 8.54e-12 2.66e-08  -8.6 2.66e-04  -4.0 1.00e+00 1.00e+00h  1\n",
+      "Reallocating memory for MA57: lfact (450838)\n",
+      "Reallocating memory for MA57: lfact (477335)\n",
+      "  23 -1.4309111e+01 3.31e-12 5.52e-09  -8.6 1.66e-04  -4.5 1.00e+00 1.00e+00h  1\n",


Do you know why you're now getting regularization at the solution when the previous run of the notebook didn't?

I added this to the todo list below. The initialization is also not as clear as I would prefer.

blnicho · 2023-08-23T07:00:19Z

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

+     "text": [
+      "INFO: =======Iteration Number: 1 =====\n",
+      "INFO: elapsed time: 5.7\n",
+      "INFO: This is the  2 run out of  9 run.\n",


I'm not sure where this info logging message is being generated but it looks like there is an off-by-one error in determining the current run number which leads to a confusing message at the end, "10 run out of 9". The grammar of this message could also be improved to "This is run 1 out of 9".

It doesn't look like your change to the log message was included when you reran the notebook. Are you running this with an installed version of Pyomo instead of the main branch?

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

jsiirola · 2023-08-23T15:38:24Z

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

+    "        )\n",
+    "\n",
+    "        # Gas constant\n",
+    "        mod.R = 8.31446261815324  # J / K / mole\n",


We should get into the habit of actually using pyo.units.

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

jsiirola · 2023-08-23T15:42:18Z

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

+    "        # Calculating C, Jacobian, FIM\n",
+    "        mod.k1_pert_rule = pyo.Constraint(mod.t, rule=cal_kp1)\n",
+    "        mod.k2_pert_rule = pyo.Constraint(mod.t, rule=cal_kp2)\n",
+    "        mod.dCdt_rule = pyo.Constraint(mod.C_set, mod.t, rule=dCdt_control)\n",
+    "\n",
+    "        mod.alge_rule = pyo.Constraint(mod.t, rule=alge)\n",


Long term, I think this example would be a lot more powerful if we could separate the kinetic model from the DoE case study. One interesting (future) direction would be to use the examples/dae/ReactionKinetics.py module to define the generic kinetic model.

I have conflicting feelings about this. In at least one case, this notebook has helped attract a new Pyomo user, so having the model in the notebook is convenient. But it also duplicates code.

…pdate_doe_tutorial

hglynch · 2023-08-23T18:30:10Z

@blnicho @jsiirola I resolved the comments from above, please let me know if there is anything else that needs to be addressed.

pyomo/contrib/doe/doe.py

blnicho

@hglynch a couple more minor comments and then I think we can merge this

pyomo/contrib/doe/doe.py

blnicho · 2023-08-23T18:41:43Z

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

+     "text": [
+      "INFO: =======Iteration Number: 1 =====\n",
+      "INFO: elapsed time: 5.7\n",
+      "INFO: This is the  2 run out of  9 run.\n",


It doesn't look like your change to the log message was included when you reran the notebook. Are you running this with an installed version of Pyomo instead of the main branch?

pyomo/contrib/doe/examples/fim_doe_tutorial.ipynb

adowling2 · 2023-08-23T19:00:14Z

Enhancements for the next PR:

Why restoration at the end of this simple problem?
Use pyomo.units for this example
Discuss keeping the model here versus importing
Why is the initial point not well initialized?

hglynch · 2023-08-23T19:16:43Z

@blnicho @jsiirola Comments should all be addressed now, let me know if there are any more.

jsiirola · 2023-08-23T19:25:59Z

@hglynch: it looks like you may have forgotten to commit/push the changes to pyomo/contrib/doe/doe.py?

hglynch · 2023-08-23T19:31:26Z

@hglynch: it looks like you may have forgotten to commit/push the changes to pyomo/contrib/doe/doe.py?

Should be working now, let me know if not!

Updated Pyomo.DoE tutorial notebook.

201ef30

jsiirola reviewed Jun 27, 2023

View reviewed changes

updates to Pyomo.DoE tutorial

44d0b6d

hglynch requested a review from adowling2 August 6, 2023 05:13

adowling2 reviewed Aug 9, 2023

View reviewed changes

hglynch changed the title ~~[WIP] Updated Pyomo.DoE tutorial notebook.~~ Updated Pyomo.DoE tutorial notebook. Aug 19, 2023

PR Comments

fe48d69

hglynch requested review from adowling2 and jsiirola August 19, 2023 03:46

adowling2 approved these changes Aug 21, 2023

View reviewed changes

notebook runs in Colab now

eb0f76b

mrmundt requested changes Aug 21, 2023

View reviewed changes

Merge branch 'main' into update_doe_tutorial

cd3f901

fixed typos

fe545e1

Fix last typo

40c32d2

mrmundt approved these changes Aug 23, 2023

View reviewed changes

mrmundt added the AT: PRE-TEST INSPECTED label Aug 23, 2023

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

blnicho reviewed Aug 23, 2023

View reviewed changes

jsiirola reviewed Aug 23, 2023

View reviewed changes

hglynch added 2 commits August 23, 2023 14:13

fixed grammar

20ff1a7

Merge branch 'update_doe_tutorial' of github.com:hglynch/pyomo into u…

7036d5b

…pdate_doe_tutorial

mrmundt added the AT: PRE-TEST INSPECTED label Aug 23, 2023

jsiirola reviewed Aug 23, 2023

View reviewed changes

pyomo/contrib/doe/doe.py Outdated Show resolved Hide resolved

blnicho reviewed Aug 23, 2023

View reviewed changes

fixed typos

6cff664

fixed typos

0f43ae9

blnicho approved these changes Aug 23, 2023

View reviewed changes

blnicho added AT: PRE-TEST INSPECTED and removed AT: PRE-TEST INSPECTED labels Aug 23, 2023

jsiirola approved these changes Aug 23, 2023

View reviewed changes

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

Update doe.py to fix black formatting

c9fd60b

mrmundt added the AT: PRE-TEST INSPECTED label Aug 23, 2023

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

blnicho merged commit 5a56343 into Pyomo:main Aug 23, 2023
25 checks passed

Updated Pyomo.DoE tutorial notebook. #2889

Updated Pyomo.DoE tutorial notebook. #2889

Conversation

hglynch commented Jun 27, 2023

Fixes # .

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

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 commented Jun 27, 2023

hglynch commented Aug 6, 2023

blnicho commented Aug 8, 2023

adowling2 left a comment

Choose a reason for hiding this comment

adowling2 commented Aug 9, 2023

mrmundt commented Aug 15, 2023 • edited Loading

adowling2 commented Aug 15, 2023

adowling2 commented Aug 18, 2023 • edited Loading

hglynch commented Aug 19, 2023

adowling2 left a comment

Choose a reason for hiding this comment

adowling2 commented Aug 21, 2023

hglynch commented Aug 21, 2023

adowling2 commented Aug 21, 2023

hglynch commented Aug 21, 2023

adowling2 commented Aug 21, 2023

hglynch commented Aug 21, 2023

Choose a reason for hiding this comment

mrmundt commented Aug 21, 2023

hglynch commented Aug 22, 2023

mrmundt commented Aug 22, 2023

hglynch commented Aug 22, 2023

mrmundt left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 23, 2023

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

blnicho Aug 23, 2023 • edited Loading

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

hglynch commented Aug 23, 2023

blnicho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adowling2 commented Aug 23, 2023

hglynch commented Aug 23, 2023

jsiirola commented Aug 23, 2023

hglynch commented Aug 23, 2023

mrmundt commented Aug 15, 2023 •

edited

Loading

adowling2 commented Aug 18, 2023 •

edited

Loading

blnicho Aug 23, 2023 •

edited

Loading