Pyomo.DoE: Correct A optimality #3803
Conversation
…mest and test_examples.py files.
…he experiment class to compute the parameters. it is now fixed.
…g test in test_examples.py 2. Fix the index issue in test_parmest. New column mismathc issue arises.
…d the test is successful except "FileNotFoundError: [Errno 2] No such file or directory: 'mpiexec'"
This reverts commit 108b9b7.
…and changed the file accordingly.
… pairplot. Now the parmest tests runs fine.
…periment constructor.
…he skipping issue
… in test_parmest.
Co-authored-by: Bethany Nicholson <[email protected]>
…ample did not solve to the desired point using A-optimality. However, when initialized to small value `1e-5`, it solved to the optimal point.
…scheme, the MM model solved without initialization from outside.
…`doe/tests`` files to reflect the changes.
…n `doe/test_utils.py`
…erstanding in ``doe/test_utils.py``
…and `_Cholesky_option = False`. Add flag for functions are the only used in `trace` calculation.
|
@adowling2 @blnicho This PR is ready for initial review. |
There was a problem hiding this comment.
Looks like you replaced every instance of trace with pseudo_trace in these tests. trace is still a valid objective type so I'm curious why you wouldn't want a mix of tests with both options.
There was a problem hiding this comment.
I have not done any testing for the new trace(trace of the covariance matrix). The previous Pyomo.DoE versions that used trace actually meant trace of Fisher Information Matrix (FIM). Now, following Dan's Greybox paper, we are referring to the trace of Fisher Information Matrix (FIM) as pseudo_trace. That's why I have replaced all instances of trace with pseudo_trace. We need new tests for the new trace (trace of the covariance matrix). I plan to do that after the review. Does this answer your question? It may be confusing.
There was a problem hiding this comment.
Thanks for the clarification. I think we should include those tests in this PR before it gets merged.
There was a problem hiding this comment.
I will add tests for the new trace (trace of the covariance matrix). This PR is for initial review to determine whether I need to make any major changes before I proceed with the testing. Thanks for reviewing.
Replace `model` with `m` in `cholesky_LLinv_imp()` Co-authored-by: Bethany Nicholson <[email protected]>
blnicho
left a comment
blnicho
left a comment
There was a problem hiding this comment.
@smondal13 thanks for addressing my initial review comments. I think these changes look reasonable so far but would like to see tests for the corrected A-optimality before I'll approve the PR.
… and add `doe_example..py` based on the rooney_biegler.py
…gler/doe_example.py`
There was a problem hiding this comment.
@blnicho @adowling2 I have created a DoE example for the rooney_biegler experiment, mainly for testing DoE. Where should this live? I can move it to parameter_estimation_example.py and rename the file to rooney_biegler_example.py or something similar. Or are you okay with the current standalone script?
|
@blnicho @adowling2 I have added tests for A-optimality and added tests for the plotting functionality in Caution: I have merged PR Parmest: consolidate rooney biegler #3793 into this branch. Therefore,Parmest: consolidate rooney biegler #3793 needs to be merged before this one. |
3 participants
Fixes
Summary/Motivation:
-In the old definition of A-optimality (we will call it pseudo A-optimality from now on), DoE computed the trace of the Fisher Information Matrix (FIM), as few studies used that definition. However, that is not correct, and new literature mentions the trace of the inverse of the FIM as A-optimality. This PR has adopted this new definition of A-optimality in DoE.
Changes proposed in this PR:
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: