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.

sscini · 2026-01-27T19:19:00Z

@adowling2 @slilonfe5 @blnicho Ready for design review.

Still adjusting tests to fix final remaining issues. Current tests had Params for experimental outputs, which I am switching to Vars and adding a Constraint instead of Expression.

Planning to restructure reaction kinetics example in a separate PR.

…o Q_opt-redesign

Did not work for multi-index like in PDEs. This addresses that.

sscini · 2026-02-05T15:49:35Z

@adowling2 @blnicho All checks passing as of last run. Please review when available. If possible to make it for this release it would be greatly appreciated, but there are many design changes. Thank you!

sscini · 2026-02-06T16:19:03Z

For reviewers: All feedback appreciated, some key areas:

Potentially missing or needed tests for modified _Q_opt implement
I left in "initialize_parmest_model" option in objective_at_theta, but it currently does nothing. Does it still need to be tested until fully removed?
Measurement error is required for using cov_est, or parmest with calc_cov = True. This is a new constraint on the user. Can this requirement be added, and if not provided, use deprecatedEstimator? Or what is best approach? Other idea is adding helper functions to Experiment class, but not sure if needed in this PR.
Recommended way to clean up imports at top of parmest.py

adowling2

Please see the comments below. There are a few places where I think we want to double check the math. @slilonfe5 and @smondal13, can you also please take a look.

adowling2 · 2026-02-06T19:31:48Z

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

@@ -55,21 +55,21 @@ def simple_reaction_model(data):
    model.k.fix()

    # ===================================================================


Can all of this commented-out code just get deleted?

adowling2 · 2026-02-06T19:32:44Z

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


This question still stands.

adowling2 · 2026-02-06T19:33:17Z

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

 from pyomo.contrib.parmest.experiment import Experiment


 def reactor_design_model():


Should this inherit from the Experiment class?

Example working normally. Intention was to update the examples in a separate PR. Following recommendation, I will complete now in this PR.

adowling2 · 2026-02-06T19:34:57Z

pyomo/contrib/parmest/tests/test_parmest.py

            columns=["hour", "y"],
        )

+        # Updated models to use Vars for experiment output, and Constraints


How does this fit in with @smondal13 's effort to consolidate all of the copies of the RB examples?

(@smondal13 please confirm) The rooney_biegler example consolidation was primarily to remove copies of the model used normally. The purpose of this test is to change the model definition (unknown parameters are provided in six different ways) to make sure these options are supported, primarily in objective_at_theta.

Please let me know if you recommend the removal or simplification of this test.

pyomo/contrib/parmest/tests/test_parmest.py

adowling2 · 2026-02-06T19:42:23Z

pyomo/contrib/parmest/parmest.py

+            # Check cov_n argument is set correctly
+            # Needs to be provided
+            assert cov_n is not NOTSET, (
+                "The number of data points 'cov_n' must be provided to calculate "


Is this something we can infer from the provided data?

Yes, we do infer it from the provided data (see _count_total_experiments()), but from the previous implementation it was required for the user to provide both calc_cov = True, and a value for cov_n. The value provided for cov_n is not currently being used, other than to check length in comparison to number of parameters.

Goal was to not modify current user experience. Is the recommendation to remove the requirement?

adowling2 · 2026-02-06T19:45:25Z

pyomo/contrib/parmest/parmest.py

@@ -1186,11 +1314,18 @@ def _cov_at_theta(self, method, solver, step):
        if method == CovarianceMethod.reduced_hessian.value:


What does this method implicitly assume? I think it assumes the user has specified the objective as the log-likelihood function plus a constant.

All methods within _cov_at_theta only support the provided weighted SSE and SSE objectives. A user specified method is currently not supported, and causes an error.

adowling2 · 2026-02-06T19:46:37Z

pyomo/contrib/parmest/parmest.py

            )

            self.inv_red_hes = inv_red_hes
+        else:


What are the other covariance methods that correspond with this else?

(@slilonfe5 confirm please) inv_red_hessian uses the ef_nonants, or the unknown parameter variables from the extended form model, and needs to be run after parameter estimation. _k_aug and finite difference use the model directly from the Experiment class.

'automatic_differentiation_k_aug' and 'finite_difference' are after the else.

pyomo/contrib/parmest/parmest.py

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 7 commits January 27, 2026 13:55

Adjusting parmest models, in progress

c248c74

Added more description, simplified comparison

b975089

Update parameter_estimation_example.py

a63e4fc

Ran black

b92aa7d

Adjusted if statement

471dbe7

Removed answered questions

98d91fc

Merge branch 'Pyomo:main' into Q_opt-redesign

a6821ae

sscini moved this from Development to Ready for design review in ParmEst & Pyomo.DoE Development Jan 27, 2026

sscini requested review from adowling2 and slilonfe5 January 27, 2026 19:47

sscini added 13 commits February 2, 2026 15:43

Update parmest.py

f0ef6d6

Fixed models in variants test

1f86b03

Merge branch 'Pyomo:main' into Q_opt-redesign

72b4345

Merge remote-tracking branch 'refs/remotes/origin/Q_opt-redesign' int…

c495e0f

…o Q_opt-redesign

Update test_parmest.py

82ee4ce

Update parmest.py

559a900

Update parmest.py

65067d5

Temporary remove failing test.

db26533

Fixed experiment counter

c9b19d7

Modified testing

7bba006

Removed extra print statements

2cd2614

Updated error message.

5ba4fab

Adjusted experimental counter

ee57ec9

Did not work for multi-index like in PDEs. This addresses that.

Update test_examples.py

7cffb34

adowling2 suggested changes Feb 6, 2026

View reviewed changes

sscini added 2 commits February 6, 2026 15:20

Addressed comments

ebbd279

Merge branch 'Pyomo:main' into Q_opt-redesign

0c1e605

		@@ -55,21 +55,21 @@ def simple_reaction_model(data):
		model.k.fix()

		# ===================================================================

		from pyomo.contrib.parmest.experiment import Experiment


		def reactor_design_model():

		@@ -1186,11 +1314,18 @@ def _cov_at_theta(self, method, solver, step):
		if method == CovarianceMethod.reduced_hessian.value:

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

Uh oh!

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!

sscini commented Jan 27, 2026

Uh oh!

sscini commented Feb 5, 2026

Uh oh!

sscini commented Feb 6, 2026

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!

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!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov bot commented Nov 30, 2025 •

edited

Loading