-
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
base: main
Are you sure you want to change the base?
Changes from 20 commits
5e99540
191ef45
5470b1d
d871e08
d588397
6797a90
6f20842
d889cc6
64a22ee
35e4b85
2ab8041
5f5d0cc
7497df6
a0005ec
b9ca381
a1770cb
f0fa351
dc40a41
ec3d9c4
eabb32f
3c9e291
8b2f487
ff56322
16345be
96973f3
9cc3111
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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?
Member
Author
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. I like this suggestion. I am going to switch to |
||
| 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"], | ||
|
|
||
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.pyThere 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.
Good catch