Finish Pyomo.DoE GreyBox (ME-opt Hessian)#3740
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3740 +/- ##
==========================================
+ Coverage 89.31% 89.33% +0.02%
==========================================
Files 896 896
Lines 103687 103752 +65
==========================================
+ Hits 92609 92688 +79
+ Misses 11078 11064 -14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixed typo in comment Co-authored-by: Bethany Nicholson <[email protected]>
It looks like my one additional test didn’t make it in a commit, I will add this shortly. It will naturally all be covered because we call the jacobian and hessian methods at each solve, but I do have a check hessian for the condition number. My next commit(s) will include this. I must have not pushed them with the PR. |
|
Also, while checking over this PR to make sure it made sense, I found a small bug in the Hessian computation. Please hold off on this one until I add more commits. Apologies @blnicho. |
blnicho
left a comment
blnicho
left a comment
There was a problem hiding this comment.
I have one minor question but otherwise I think this looks fine.
| elif self.objective_option == ObjectiveLib.condition_number: | ||
| cond_number = np.linalg.cond(np.array(self.get_FIM())) | ||
| eig, _ = np.linalg.eig(np.array(self.get_FIM())) | ||
| cond_number = np.log(np.abs(np.max(eig) / np.min(eig))) |
There was a problem hiding this comment.
Are you going to cause any confusion with users by returning the log of the condition number and not the condition number directly? Should ObjectiveLib.condition_number be replaced with ObjectiveLib.log_condition_number?
There was a problem hiding this comment.
The answer is maybe. The determinant option also returns the log of the determinant for scaling purposes, so I think this will not come as too much of a surprise.
I will make sure that this is clear when the documentation is added.
github-project-automation
bot
moved this from Review In Progress
to Reviewer Approved
in Pyomo 6.9.5 Release
Co-authored-by: Bethany Nicholson <[email protected]>
github-project-automation
bot
moved this from Reviewer Approved
to Done
in Pyomo 6.9.5 Release
6 participants
Fixes # .
Added code to compute the Hessian in the condition number number case (ME-opt) instead of using
pass. Also, update condition number objective to be log-scale to avoid numerical issues.Summary/Motivation:
Numerical issues caused a delay for adding this. We figured out the numerical issues and this is good to go from the math side.
Changes proposed in this PR:
condition numberoption for Pyomo.DoE using GreyBox objective use the log of the condition numbercondition numberoption (instead ofpassbeing used)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: