Simplify _Q_opt in parmest with block scenario structure. #3789

sscini · 2025-11-20T16:57:51Z

Fixes # .

Summary/Motivation:

_Q_opt, the main model building and solving function within parmest, is still heavily dependent on MSISPPY, and the code is becoming outdated and difficult to interpret. The goal of this PR is to redesign _Q_opt to work without this dependence, retaining all the current functionality

Changes proposed in this PR:

-_Q_opt redesign, using a block structure, with minimal changes needed to functions that utilize it.

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.

… Q_opt-redesign

sscini · 2025-11-20T16:59:01Z

@djlaky @adowling2 Here are my initial thoughts on the redesign. Please provide feedback on code layout to this point.

sscini · 2025-11-20T21:22:06Z

Dan and I had a good conversation today about the functional things needed in the redesign, and how to go about adding components to the overall block and sub-models. Working to implement changes, closely related to what is done currently in doe's _generate_scenario_blocks.

adowling2

Nice progress

pyomo/contrib/parmest/parmest.py

sscini · 2025-11-25T17:34:02Z

@adowling2 @djlaky Added case for bootstrap, now works with theta_est, theta_est_bootstrap, and theta_est_leaveNout if I replace _Q_opt with _Q_opt_blocks. Please review.

I am aware of the duplicated code, but when I attempted to unify them using n_scenarios = len(self.exp_list) or len(bootlist), I am getting an error for bootstrap I am still working out.

Would we want to fully transition to using theta_est for obj and theta, and cov_est for covariance and add an error/warning? Or should I make it work to calculate cov if calc_cov=True?

Thanks!

codecov · 2025-11-30T19:59:10Z

Codecov Report

❌ Patch coverage is 4.76190% with 100 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.31%. Comparing base (13facca) to head (d91ce3f).
⚠️ Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
pyomo/contrib/parmest/parmest.py	4.76%	100 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3789      +/-   ##
==========================================
- Coverage   89.40%   89.31%   -0.10%     
==========================================
  Files         909      909              
  Lines      105541   105646     +105     
==========================================
- Hits        94364    94360       -4     
- Misses      11177    11286     +109

Flag	Coverage Δ
builders	`29.07% <4.76%> (-0.04%)`	⬇️
default	`85.94% <4.76%> (?)`
expensive	`35.73% <4.76%> (?)`
linux	`86.63% <4.76%> (-2.55%)`	⬇️
linux_other	`86.63% <4.76%> (-0.10%)`	⬇️
osx	`82.78% <4.76%> (-0.09%)`	⬇️
win	`84.88% <4.76%> (-0.09%)`	⬇️
win_other	`84.88% <4.76%> (-0.09%)`	⬇️

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

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sscini · 2025-12-09T17:36:48Z

@adowling2 @djlaky Should I tag larger team on this? Please review when available. Thanks!

…ocks" This reverts commit 1aea99f.

…_list) cov_est() needs the experiment class to have variables for params, which makes a few tests fail. Was failing tests with one experiment.

adowling2

@sscini Here is my first pass at the review. A handful of places would benefit from more verbose comments to explain the logic or assumptions.

I wonder if there is a better way besides getattr to access variable values. I am thinking how we access values in Pyomo.DoE for variables that are in a suffix.

@slilonfe5, please take a look. I think this would benefit from a conversation with you to integrate the different methods to estimate the parameter covariance.

adowling2 · 2026-01-20T17:44:35Z

pyomo/contrib/parmest/examples/reaction_kinetics/simple_reaction_parmest_example.py

    model.x2 = Param(initialize=float(data['x2']))

    # Rate constants
+    # @Reviewers: Can we switch this to explicitly defining which parameters are to be


Yes, I think this example could/should be reworked to fully embrace the Experiment class.

adowling2 · 2026-01-20T17:47:11Z

pyomo/contrib/parmest/examples/reactor_design/parameter_estimation_example.py


    # Parameter estimation with covariance
-    obj, theta, cov = pest.theta_est(calc_cov=True, cov_n=17)
+    obj, theta, cov = pest.theta_est(calc_cov=True, cov_n=19)


Just to satisfy my curiosity, is 17 incorrect and 19 correct? Is this a mistake in the example that is being fixed?

From the data in the csv file, it should be 19. There are 19 experiments and 4 experiment outputs. The experiment counter (_count_total_experiments) is currently returning 76, which is why I have mentioned making it more robust and commented out the related assertion at this time.

adowling2 · 2026-01-20T17:47:58Z

pyomo/contrib/parmest/examples/reactor_design/parameter_estimation_example.py

    # Parameter estimation with covariance
-    obj, theta, cov = pest.theta_est(calc_cov=True, cov_n=17)
+    obj, theta, cov = pest.theta_est(calc_cov=True, cov_n=19)
    print(obj)


While we are editing this, let's print out something such as "print("Least squares objective = ",obj)", etc. to help a new user.

adowling2 · 2026-01-20T18:06:28Z

pyomo/contrib/parmest/examples/reactor_design/reactor_design.py

-    )  # min^-1
-    model.k3 = pyo.Param(
-        initialize=1.0 / 6000.0, within=pyo.PositiveReals, mutable=True
+    # Rate constants, make unknown parameters variables


Should this example get refactored to use the Experiment class? Is this something @smondal13 is thinking about for a separate PR?

adowling2 · 2026-01-20T18:08:06Z

pyomo/contrib/parmest/parmest.py

 #### Redesign with Experiment class Dec 2023

+# Options for using mpi-sppy or local EF only used in the deprecatedEstimator class
 # TODO: move use_mpisppy to a Pyomo configuration option


Does this need to get cleaned up? Is this for the deprecated interface only?

Will be cleaned up. Yes, currently the use_mpisppy option is only included in deprecated interface. This was added before I started this PR.

adowling2 · 2026-01-20T18:32:23Z

pyomo/contrib/parmest/parmest.py

+            for exp_i in exp_blocks:
+                vals = {}
+                for var in return_values:
+                    exp_i_var = exp_i.find_component(str(var))


What is this doing? More comments would help? Is it assuming specific things about the structure of the parmest model? At a minimum, I would recommend documenting any assumptions in comments at least.

adowling2 · 2026-01-20T18:33:36Z

pyomo/contrib/parmest/parmest.py

+            var_values = pd.DataFrame(var_values)
+
+        # Calculate covariance if requested using cov_est()
+        if calc_cov is not NOTSET and calc_cov:


When would calc_cov be assigned NOTSET?

[Addresses next few comments]. Need to confirm with @blnicho and team which tests need to be kept and enforced with these changes. If we call cov_est or _cov_at_theta here, then we are restraining the user to only using variables for unknown parameters, which makes multiple of the existing tests and examples fail. The inverse reduced hessian method was what @slilonfe5 implemented to calculate the covariance if a user still wishes to use theta_est for covariance. Previous conversations indicated these features should be preserved from my understanding.
Will also add more comments

If the user doesn't pass the calc_cov and cov_n arguments in theta_est, then calc_cov is NOTSET and cov_n is NOTSET.

adowling2 · 2026-01-20T18:35:23Z

pyomo/contrib/parmest/parmest.py

-                return obj_val, theta_vals, cov
-            elif calc_cov is NOTSET or not calc_cov:
-                return obj_val, theta_vals
+            measurement_var = self.obj_value / (


How does this work with the changes @slilonfe5 introduced to define different objectives?

@sscini, please see my comments above for answers

adowling2 · 2026-01-20T18:36:54Z

pyomo/contrib/parmest/parmest.py

-                    return obj_val, theta_vals, var_values, cov
-                elif calc_cov is NOTSET or not calc_cov:
-                    return obj_val, theta_vals, var_values
+            solve_result, inv_red_hes = (


Should this call the _cov_at_theta function?

I agree @adowling2. @sscini, to get the covariance matrix when the user employs the old method of theta_est, instead of duplicating code, you should get the covariance matrix from cov_est which uses _cov_at_theta function (note _cov_at_theta is not related to and does perform similar tasks as _Q_at_theta, you can view or rename it as _cov_opt). E.g., cov = cov_est(method="reduced_hessian"). For covariance calculation using the legacy theta_est, use the "reduced_hessian" method.

@slilonfe5 I had this implemented. Main concern here was this caused model variants tests to fail (e.g. Params, indexedParams) as we discussed. I will look at extracting the values from the parmest model and review the change with you to fix this.

@sscini I'm curious as to why they are failing, given that previously, when we've been using the "reduced_hessian" method to compute covariance using theta_est, those tests did not fail. If it's methods like "finite_difference", I will understand that the parameters need to be defined with Var.

If I run the tests on the current version of parmest that I have, those tests do not fail, and they also use the "reduced_hessian" method. Maybe something else is causing them to fail.

adowling2 · 2026-01-20T18:38:11Z

pyomo/contrib/parmest/parmest.py

                    1
                )  # initialization performed using just 1 set of theta values
+
+        # print("DEBUG objective_at_theta_blocks")


Remove these commented out print statements before we merge. They are okay to keep while debugging / working on other feedback.

slilonfe5 · 2026-01-20T19:02:37Z

@sscini Here is my first pass at the review. A handful of places would benefit from more verbose comments to explain the logic or assumptions.

I wonder if there is a better way besides getattr to access variable values. I am thinking how we access values in Pyomo.DoE for variables that are in a suffix.

@slilonfe5, please take a look. I think this would benefit from a conversation with you to integrate the different methods to estimate the parameter covariance.

Hello @adowling2, I agree. Stephen and I have been talking a lot about integrating the covariance estimation, as well as help with understanding the parmest test file. I will review and leave comments and suggestions.

sscini · 2026-01-20T19:06:18Z

Notes to discuss in 01/20/26 meeting:

Does the use of parameters and indexed parameters still need to be preserved?
The _count_total_experiments function: Suggestions for path forward.

slilonfe5 · 2026-01-20T19:16:11Z

Notes to discuss in 01/20/26 meeting:

Does the use of parameters and indexed parameters still need to be preserved? The _count_total_experiments function: Suggestions for path forward.

Hello @sscini. I think you want your code to be robust to those old parmest examples, we can discuss later. I will leave suggestions on the _count_total_experiments function.

pyomo/contrib/parmest/parmest.py

slilonfe5 · 2026-01-21T17:05:05Z

pyomo/contrib/parmest/parmest.py

+                model.exp_scenarios[i].transfer_attributes_from(parmest_model)
+                # model.exp_scenarios[i].pprint()
+
+        # Transfer all the unknown parameters to the parent model


Are the unknown_parameters not already defined in the Experiment class? I don't understand what you're trying to do here

slilonfe5 · 2026-01-21T17:09:03Z

pyomo/contrib/parmest/parmest.py

+                                for index in var:
+                                    val = ThetaVals[name][index]
+                                    var[index].set_value(val)
+                                    if fix_theta:


I don't understand why we are fixing theta? Isn't theta what we are solving for in the minimization problem?

I combined _Q_opt and _Q_at_theta. _Q_opt estimates, _Q_at_theta returns objective with fixed theta values (objective_at_theta). Previously, _Q_at_theta built each scenario sequentially to avoid building extended form model, and divided by number of experiments. Now it just fixes values in extended form. It is a lot less costly, and actually made objective_at_theta a lot faster. This also lets me use it, when unfixed, for multistart estimation (next PR). I will definitely add more comments about this.

slilonfe5 · 2026-01-21T17:11:15Z

pyomo/contrib/parmest/parmest.py

-            obj_val = pyo.value(ef.EF_Obj)
-            self.obj_value = obj_val
-            self.estimated_theta = theta_vals
+        model.Obj = pyo.Objective(rule=total_obj, sense=pyo.minimize)


If theta is fixed, I don't get how we are finding the optimal theta here.

Fixed theta, returns objective, previously used for model initialization. Will add more comments for logic.

slilonfe5 · 2026-01-21T17:18:50Z

pyomo/contrib/parmest/parmest.py

-                    cov, index=theta_vals.keys(), columns=theta_vals.keys()
-                )
+        # Solve model
+        solve_result = sol.solve(model, tee=self.tee)


You need to check the solve_result status here. You need the check to be global

Since I am combining, the global check is below following if not fix_theta:, where the rest of the normal _Q_opt workflow takes place. The if fix_theta is meant to separate _Q_at_theta logic, which reports the worst solver status back to the user, as long as it is not infeasible.

sscini added 4 commits November 17, 2025 21:15

First pass at code redesign, still need to figure out more

10f6b37

Merge branch 'Pyomo:main' into Q_opt-redesign

b0b7e2c

Added comments where I have question

3e95e91

Merge branch 'Q_opt-redesign' of https://github.com/sscini/pyomo into…

8d3a4d5

… Q_opt-redesign

sscini added 2 commits November 21, 2025 01:35

Got preliminary _Q_opt simple working with example!

3982e1b

Ran black

e829344

sscini added this to ParmEst & Pyomo.DoE Development Nov 21, 2025

sscini moved this to Development in ParmEst & Pyomo.DoE Development Nov 21, 2025

adowling2 reviewed Nov 21, 2025

View reviewed changes

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

sscini added 5 commits November 21, 2025 09:36

Changed name to _Q_opt_blocks

e464097

Update parmest.py

dc5ee76

Ran black

6355818

Added in case for bootlist, works with example

099f541

Ran black

76ee05e

Simplified structure, ran black

d91ce3f

jsiirola added the AT: PRE-TEST INSPECTED label Nov 30, 2025

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Nov 30, 2025

blnicho added the ParmEst label Dec 11, 2025

sscini added 7 commits December 13, 2025 15:09

Removed _Q_opt, and replicate functions, only using _Q_opt_blocks

1aea99f

Ran black on mac

7d93cc0

Revert "Removed _Q_opt, and replicate functions, only using _Q_opt_bl…

d7d2214

…ocks" This reverts commit 1aea99f.

Added testing statement

7f21344

Ran black

32d8d41

Merge branch 'Pyomo:main' into Q_opt-redesign

4e8c3ee

Made small design changes, in progress, ran black.

1e802ba

sscini added 4 commits January 19, 2026 13:17

Switched unknown_params to Vars

062a9ee

Fixed number for cov_n, still need to adjust counting function

5baaa2f

Added question for reviewers

c8194ac

Ran black

26d70e3

sscini requested a review from adowling2 January 19, 2026 18:22

sscini added 7 commits January 19, 2026 14:37

Changed retrieval of variables for ind_red_hes.

4aa027d

Added note related to count_total_experiments, commented out assertion.

26ba2ea

Ran black

dd926f8

Removed dependence on cov_est for theta_est(), added bool for len(exp…

7b70d1d

…_list) cov_est() needs the experiment class to have variables for params, which makes a few tests fail. Was failing tests with one experiment.

Merge branch 'main' into Q_opt-redesign

43cbaa4

Ran black

07798c9

Attempted adding support for indexed vars

b325f0d

adowling2 suggested changes Jan 20, 2026

View reviewed changes

sscini added 2 commits January 20, 2026 14:31

Ran black

8b47430

Addressed some review comments.

aac1476

slilonfe5 reviewed Jan 21, 2026

View reviewed changes

pyomo/contrib/parmest/parmest.py Show resolved Hide resolved

slilonfe5 reviewed Jan 21, 2026

View reviewed changes

pyomo/contrib/parmest/parmest.py Show resolved Hide resolved

slilonfe5 reviewed Jan 21, 2026

View reviewed changes

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

slilonfe5 reviewed Jan 21, 2026

View reviewed changes

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

slilonfe5 reviewed Jan 21, 2026

View reviewed changes

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

slilonfe5 reviewed Jan 21, 2026

View reviewed changes

sscini added 3 commits January 21, 2026 17:55

Merge branch 'Pyomo:main' into Q_opt-redesign

66a1396

Added Shammah fix for exp count

3957dc9

Updates to address comments.

382ea20

Simplify _Q_opt in parmest with block scenario structure. #3789

Are you sure you want to change the base?

Simplify _Q_opt in parmest with block scenario structure. #3789

Conversation

sscini commented Nov 20, 2025

Fixes # .

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

Uh oh!

sscini commented Nov 20, 2025

Uh oh!

sscini commented Nov 20, 2025

Uh oh!

adowling2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sscini commented Nov 25, 2025

Uh oh!

codecov bot commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sscini commented Dec 9, 2025

Uh oh!

adowling2 left a comment

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!

sscini Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

slilonfe5 commented Jan 20, 2026

Uh oh!

sscini commented Jan 20, 2026

Uh oh!

slilonfe5 commented Jan 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Nov 30, 2025 •

edited

Loading

sscini Jan 20, 2026 •

edited

Loading