Skip to content

Pyomo.DoE: adding more verbose output for sensitivity analysis #3525

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

Open
wants to merge 77 commits into
base: main
Choose a base branch
from

Conversation

smondal13
Copy link

@smondal13 smondal13 commented Mar 19, 2025

Fixes # .

Summary/Motivation:

  • Adding more verbose is helpful to analyze the problem
  • Adding a new method to check the FIM in the utils.py can help in the general usage of the method.
  • Adding a new standalone function in utils.py to calculate different metrics of the FIM is helpful in case the user does not want to build the get_labeled_model().
  • Adding arguments in the reactor_example.py will help with the testing.

Changes proposed in this PR:

  • added eigenvalues, determinant of FIM, and trace of FIM for sensitivity output in compute_FIM_full_factorial()
  • added check_FIM() in utils.py by separating it from check_model_FIM() in doe.py
  • added compute_FIM_metrics() in utils.py
  • added get_FIM_metrics() in utils.py
  • added arguments in the reactor_example.py

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:

  1. I agree my contributions are submitted under the BSD license.
  2. 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.

@smondal13 smondal13 marked this pull request as draft March 19, 2025 14:13
@smondal13
Copy link
Author

@adowling2 @djlaky ready for early feedback

@adowling2
Copy link
Member

Next step:

  • Also add solver status

@adowling2
Copy link
Member

@smondal13 See #3532 for an example of what @jsiirola is suggesting

… doe object that uses the "reactor_experiment" example in doe directory. Also added the png files to gitignore that are stored when "reactor_example.py" is run.
@smondal13 smondal13 force-pushed the adding_eigen_values branch from 6e68c2b to e9e83b6 Compare April 30, 2025 01:17
@github-project-automation github-project-automation bot moved this from Review In Progress to Done in July 2025 Release Jun 30, 2025
@github-project-automation github-project-automation bot moved this from Ready for final review to Done in ParmEst & Pyomo.DoE Development Jun 30, 2025
@smondal13 smondal13 reopened this Jun 30, 2025
@smondal13 smondal13 force-pushed the adding_eigen_values branch from 8e4a5fb to baa817b Compare June 30, 2025 19:21
@smondal13
Copy link
Author

@blnicho, I have added the print statement, and on my Mac, the values of "log10 D-opt" are exactly the same as the "log10_D_opt_expected" values. test_doe_solve.py passed all the tests on my machine. As per your suggestion, I have changed atol = 1e-6 from its default value, atol=1e-8. I hope this works for other operating systems.

I have also solved the import issue.

Comment on lines 515 to 517
self.assertTrue(
np.allclose(ff_results["log10 D-opt"], log10_D_opt_expected, atol=1e-6)
)
Copy link
Member

Choose a reason for hiding this comment

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

Tests are still failing due to slight numerical differences across different operating systems. Try changing these to use self.assertStructuredAlmostEqual(value, expected) instead of the combo of assertTrue and np.allclose. I think this will print out which values are different when the test fails without needing the explicit print statement you added above. You may have to use Python lists of floats instead of numpy arrays and data types. Once we figure out the tolerance to use for "log10 D-opt", I'm expecting we'll see similar numerical issues with the checks below.

Suggested change
self.assertTrue(
np.allclose(ff_results["log10 D-opt"], log10_D_opt_expected, atol=1e-6)
)
self.assertStructuredAlmostEqual(ff_results["log10 D-opt"], log10_D_opt_expected, abstol=1e-4)

Copy link
Author

@smondal13 smondal13 Jul 1, 2025

Choose a reason for hiding this comment

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

@blnicho, I have changed the design_ranges for the example in test_doe_solve.py . Previously, the values of D-opt were very small. I have also changed the datatype of the lists. These changes will probably solve the numerical issue.

smondal13 and others added 3 commits June 30, 2025 21:31
Co-authored-by: Bethany Nicholson <[email protected]>
…ged the expected values in class TestDoe from list of numpy floats to list of floats.
@blnicho
Copy link
Member

blnicho commented Jul 1, 2025

@smondal13 the lint check failed. Please run black on this again.

@blnicho blnicho moved this from Done to Reviewer Approved in July 2025 Release Jul 1, 2025
@blnicho blnicho moved this from Reviewer Approved to Review In Progress in July 2025 Release Jul 1, 2025
@smondal13
Copy link
Author

smondal13 commented Jul 2, 2025

@blnicho, I have changed the example again. This time, it has passed all the checks, except one.

@github-project-automation github-project-automation bot moved this from Review In Progress to Reviewer Approved in July 2025 Release Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Reviewer Approved
Development

Successfully merging this pull request may close these issues.

6 participants