Skip to content

Pyomo.DoE bugfixes #3574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 21, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion pyomo/contrib/doe/doe.py
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ def _sequential_FIM(self, model=None):

# Simulate the model
try:
res = self.solver.solve(model)
res = self.solver.solve(model, tee=self.tee)
pyo.assert_optimal_termination(res)
except:
# TODO: Make error message more verbose, i.e., add unknown parameter values so the
Expand Down Expand Up @@ -1119,8 +1119,17 @@ def build_block_scenarios(b, s):
pyo.ComponentUID(param, context=m.base_model).find_component_on(
b
).set_value(m.base_model.unknown_parameters[param] * (1 + diff))

# Fix experiment inputs before solve (enforce square solve)
for comp in b.experiment_inputs:
comp.fix()
Comment on lines +1123 to +1125
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any model checking before this that ensures fixing b.experiment_inputs will result in a square model?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is there any chance that a user will have already fixed some of these inputs in which case we shouldn't unfix them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is not a chance of this. The understanding of Pyomo.DoE is be that all the experiment inputs (as labeled) should be fixed during initialization and free during the optimization call. If they should be fixed, the user should not include them in that suffix.

@adowling2, can you check to make sure what I said is correct?

Copy link
Contributor Author

@djlaky djlaky Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any model checking before this that ensures fixing b.experiment_inputs will result in a square model?

There is not.

Since the blocks are built on cloning the model from 'get_labeled_model()', the model output from this call needs to be square.

Also, is there a way to tell if the model is square using the 'res' object in line 1127? That would be the best case, then the user can get an error if their model is not square and I can write a test in the suite.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of this failing gracefully if the user tries running it with a non-square model. I have less of an opinion on if the check happens here or somewhere else.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we have a custom function that inspects the build model and summarize the number of measurements, equations, parameters, inputs, etc. and estimates the degrees of freedom. I am thinking "If someone contacted me asking for help with ParmEst or Pyomo.DoE, what would I am to know about the model? I'm s there a way we can make the first debugging step to run "model_diagnostics" or similar and report the output.


res = self.solver.solve(b, tee=self.tee)

# Unfix experiment inputs after square solve
for comp in b.experiment_inputs:
comp.unfix()

model.scenario_blocks = pyo.Block(model.scenarios, rule=build_block_scenarios)

# To-Do: this might have to change if experiment inputs have
Expand Down