-
Notifications
You must be signed in to change notification settings - Fork 577
Pyomo.DoE: fix trace/Cholesky initialization consistency #3867
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
adowling2
wants to merge
20
commits into
Pyomo:main
Choose a base branch
from
adowling2:fix-doe-initialization
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
5e99540
Added prototype capabilities for debugging Pyomo.DoE initial points
adowling2 191ef45
Updated jitter calculation for FIM
adowling2 5470b1d
Updated FIM initialization
adowling2 d871e08
Improved API for run_doe
adowling2 d588397
Added missing doc strings
adowling2 6797a90
Add doc strings to the tests
adowling2 6f20842
Adds comments
adowling2 d889cc6
Dedicated example for Pyomo.DoE initial point debugging
adowling2 64a22ee
Ran Black
adowling2 35e4b85
Updated tests
adowling2 2ab8041
Improved test documentation
adowling2 5f5d0cc
Additional edits to for GreyBox in Pyomo.DoE
adowling2 7497df6
Continued to debug GreyBox
adowling2 a0005ec
Update to avoid NaN
adowling2 b9ca381
Merge branch 'main' into fix-doe-initialization
adowling2 a1770cb
Merge branch 'main' into fix-doe-initialization
adowling2 f0fa351
Removed new run_doe() interface
adowling2 dc40a41
Changed test file name
adowling2 ec3d9c4
Ran black and typos
adowling2 eabb32f
Fixed two tests
adowling2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -281,7 +281,6 @@ def run_doe(self, model=None, results_file=None): | |
| results_file: string name of the file path to save the results | ||
| to in the form of a .json file | ||
| default: None --> don't save | ||
|
|
||
| """ | ||
| # Check results file name | ||
| if results_file is not None: | ||
|
|
@@ -376,6 +375,11 @@ def run_doe(self, model=None, results_file=None): | |
| if self.objective_option == ObjectiveLib.trace: | ||
| trace_val = np.trace(np.linalg.pinv(self.get_FIM())) | ||
| model.obj_cons.egb_fim_block.outputs["A-opt"].set_value(trace_val) | ||
| elif self.objective_option == ObjectiveLib.pseudo_trace: | ||
| pseudo_trace_val = np.trace(np.array(self.get_FIM())) | ||
| model.obj_cons.egb_fim_block.outputs["pseudo-A-opt"].set_value( | ||
| pseudo_trace_val | ||
| ) | ||
| elif self.objective_option == ObjectiveLib.determinant: | ||
| det_val = np.linalg.det(np.array(self.get_FIM())) | ||
| model.obj_cons.egb_fim_block.outputs["log-D-opt"].set_value( | ||
|
|
@@ -389,63 +393,8 @@ def run_doe(self, model=None, results_file=None): | |
| cond_number = np.log(np.abs(np.max(eig) / np.min(eig))) | ||
| model.obj_cons.egb_fim_block.outputs["ME-opt"].set_value(cond_number) | ||
|
|
||
| # If the model has L, initialize it with the solved FIM | ||
| if hasattr(model, "L"): | ||
| # Get the FIM values | ||
| fim_vals = [ | ||
| pyo.value(model.fim[i, j]) | ||
| for i in model.parameter_names | ||
| for j in model.parameter_names | ||
| ] | ||
| fim_np = np.array(fim_vals).reshape( | ||
| (len(model.parameter_names), len(model.parameter_names)) | ||
| ) | ||
|
|
||
| # Need to compute the full FIM before | ||
| # initializing the Cholesky factorization | ||
| if self.only_compute_fim_lower: | ||
| fim_np = fim_np + fim_np.T - np.diag(np.diag(fim_np)) | ||
|
|
||
| # Check if the FIM is positive definite | ||
| # If not, add jitter to the diagonal | ||
| # to ensure positive definiteness | ||
| min_eig = np.min(np.linalg.eigvals(fim_np)) | ||
|
|
||
| if min_eig < _SMALL_TOLERANCE_DEFINITENESS: | ||
| # Raise the minimum eigenvalue to at | ||
| # least _SMALL_TOLERANCE_DEFINITENESS | ||
| jitter = np.min( | ||
| [ | ||
| -min_eig + _SMALL_TOLERANCE_DEFINITENESS, | ||
| _SMALL_TOLERANCE_DEFINITENESS, | ||
| ] | ||
| ) | ||
| else: | ||
| # No jitter needed | ||
| jitter = 0 | ||
|
|
||
| # Add jitter to the diagonal to ensure positive definiteness | ||
| L_vals_sq = np.linalg.cholesky( | ||
| fim_np + jitter * np.eye(len(model.parameter_names)) | ||
| ) | ||
| for i, c in enumerate(model.parameter_names): | ||
| for j, d in enumerate(model.parameter_names): | ||
| model.L[c, d].value = L_vals_sq[i, j] | ||
|
|
||
| # Initialize the inverse of L if it exists | ||
| if hasattr(model, "L_inv"): | ||
| L_inv_vals = np.linalg.inv(L_vals_sq) | ||
|
|
||
| for i, c in enumerate(model.parameter_names): | ||
| for j, d in enumerate(model.parameter_names): | ||
| if i >= j: | ||
| model.L_inv[c, d].value = L_inv_vals[i, j] | ||
| else: | ||
| model.L_inv[c, d].value = 0.0 | ||
| # Initialize the cov_trace if it exists | ||
| if hasattr(model, "cov_trace"): | ||
| initial_cov_trace = np.sum(L_inv_vals**2) | ||
| model.cov_trace.value = initial_cov_trace | ||
| # Keep Cholesky-related variables synchronized with current FIM values | ||
| self._initialize_cholesky_from_fim(model=model) | ||
|
|
||
| if hasattr(model, "determinant"): | ||
| model.determinant.value = np.linalg.det(np.array(self.get_FIM())) | ||
|
|
@@ -537,6 +486,105 @@ def run_doe(self, model=None, results_file=None): | |
| with open(results_file, "w") as file: | ||
| json.dump(self.results, file) | ||
|
|
||
| def _compute_cholesky_jitter(self, min_eig): | ||
| """ | ||
| Compute diagonal regularization for Cholesky initialization. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| min_eig: float | ||
| Minimum eigenvalue of the current FIM estimate. | ||
|
|
||
| Returns | ||
| ------- | ||
| float | ||
| Nonnegative diagonal shift needed so the minimum eigenvalue | ||
| is at least ``_SMALL_TOLERANCE_DEFINITENESS``. | ||
| """ | ||
| return max(0.0, _SMALL_TOLERANCE_DEFINITENESS - float(min_eig)) | ||
|
|
||
| def _get_fim_numpy(self, model): | ||
| """ | ||
| Assemble the current FIM variable values into a NumPy array. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| model: ConcreteModel | ||
| DoE model containing variable ``fim``. | ||
|
|
||
| Returns | ||
| ------- | ||
| ndarray | ||
| Dense FIM array. If ``only_compute_fim_lower`` is True, the | ||
| returned array is symmetrized from the lower triangle. | ||
| """ | ||
| fim_vals = [ | ||
| pyo.value(model.fim[i, j]) | ||
| for i in model.parameter_names | ||
| for j in model.parameter_names | ||
| ] | ||
| fim_np = np.array(fim_vals, dtype=float).reshape( | ||
| (len(model.parameter_names), len(model.parameter_names)) | ||
| ) | ||
| if self.only_compute_fim_lower: | ||
| fim_np = fim_np + fim_np.T - np.diag(np.diag(fim_np)) | ||
| return fim_np | ||
|
|
||
| def _initialize_cholesky_from_fim(self, model=None): | ||
| """ | ||
| Synchronize Cholesky-related variables using the current FIM. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| model: ConcreteModel, optional | ||
| DoE model to update. Defaults to ``self.model``. | ||
|
|
||
| Returns | ||
| ------- | ||
| None | ||
| Updates model values in place for available variables: | ||
| ``L``, ``L_inv``, ``fim_inv``, and ``cov_trace``. | ||
| """ | ||
| if model is None: | ||
| model = self.model | ||
| if not hasattr(model, "L"): | ||
| return | ||
|
|
||
| fim_np = self._get_fim_numpy(model) | ||
| min_eig = float(np.min(np.linalg.eigvalsh(fim_np))) | ||
| jitter = self._compute_cholesky_jitter(min_eig) | ||
| fim_pd = fim_np + jitter * np.eye(len(model.parameter_names)) | ||
|
|
||
| L_vals = np.linalg.cholesky(fim_pd) | ||
| for i, c in enumerate(model.parameter_names): | ||
| for j, d in enumerate(model.parameter_names): | ||
| if i >= j: | ||
| model.L[c, d].value = L_vals[i, j] | ||
| else: | ||
| model.L[c, d].value = 0.0 | ||
|
|
||
| if hasattr(model, "L_inv"): | ||
| L_inv_vals = np.linalg.inv(L_vals) | ||
| for i, c in enumerate(model.parameter_names): | ||
| for j, d in enumerate(model.parameter_names): | ||
| if i >= j: | ||
| model.L_inv[c, d].value = L_inv_vals[i, j] | ||
| else: | ||
| model.L_inv[c, d].value = 0.0 | ||
|
|
||
| if hasattr(model, "fim_inv"): | ||
| fim_inv_vals = np.linalg.inv(fim_pd) | ||
| for i, c in enumerate(model.parameter_names): | ||
| for j, d in enumerate(model.parameter_names): | ||
| if self.only_compute_fim_lower and i < j: | ||
| model.fim_inv[c, d].value = 0.0 | ||
| else: | ||
| model.fim_inv[c, d].value = fim_inv_vals[i, j] | ||
|
|
||
| if hasattr(model, "cov_trace"): | ||
| fim_inv_np = np.linalg.inv(fim_pd) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you are making the fim PD by adding the jitter, I believe the fim_pd will probably be non-singular. But if it is singular for some edge cases, then isn't it better to use the pseudo-inverse here just to be safe? |
||
| model.cov_trace.value = np.trace(fim_inv_np) | ||
|
|
||
| # Perform multi-experiment doe (sequential, or ``greedy`` approach) | ||
| def run_multi_doe_sequential(self, N_exp=1): | ||
| raise NotImplementedError("Multiple experiment optimization not yet supported.") | ||
|
|
@@ -898,6 +946,7 @@ def create_doe_model(self, model=None): | |
| self.only_compute_fim_lower | ||
| and self.objective_option == ObjectiveLib.determinant | ||
| and not self.Cholesky_option | ||
| and not self.use_grey_box | ||
| ): | ||
| raise ValueError( | ||
| "Cannot compute determinant with explicit formula " | ||
|
|
@@ -1663,6 +1712,11 @@ def FIM_egb_cons(m, p1, p2): | |
| model.objective = pyo.Objective( | ||
| expr=model.obj_cons.egb_fim_block.outputs["A-opt"], sense=pyo.minimize | ||
| ) | ||
| elif self.objective_option == ObjectiveLib.pseudo_trace: | ||
| model.objective = pyo.Objective( | ||
| expr=model.obj_cons.egb_fim_block.outputs["pseudo-A-opt"], | ||
| sense=pyo.maximize, | ||
| ) | ||
| elif self.objective_option == ObjectiveLib.determinant: | ||
| model.objective = pyo.Objective( | ||
| expr=model.obj_cons.egb_fim_block.outputs["log-D-opt"], | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not see any tests for the
pseudo_traceobjective intest_greybox.py