Skip to content

Conversation

bammari
Copy link

@bammari bammari commented Sep 5, 2025

Fixes

We generalize the OMLTSurrogate base class to include models other than Keras models. We refactor these changes across the KerasSurrogate class as well. We do this because future interfaces to OMLT models may inherity from the OMLTSurrogate base class without being specific to Keras. These are mostly semantic changes.

Summary/Motivation:

We add an interface to the OMLT linear-tree capabilities to allow users to implement Linear Model Decision Trees as surrogates within their IDAES models.

Changes proposed in this PR:

  • Generalize OMLTSurrogate
  • Add LinearTreeSurrogate class which inherits from OMLTSurrogate
  • Add tests to test this new interface.

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  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.

@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Sep 11, 2025
Copy link
Contributor

@bpaul4 bpaul4 left a comment

Choose a reason for hiding this comment

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

These changes look reasonable to me, just a couple of very minor "missing periods" in docstrings but feel free to ignore. I will update the branch and approve the workflow.

This constructor should only be used when first creating the surrogate within IDAES.
Once created, this object can be stored to disk using save_to_folder and loaded
with load_from_folder
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
with load_from_folder
with load_from_folder.

Comment on lines +64 to +77
Args:
lt_model: Linear-tree model
This is the Linear-tree model that will be loaded.
input_labels: list of str
The ordered list of labels corresponding to the inputs in the linear-tree model
output_labels: list of str
The ordered list of labels corresponding to the outputs in the linear-tree model
input_bounds: None of dict of tuples
Keys correspond to each of the input labels and values are the tuples of
bounds (lb, ub)
input_scaler: None or OffsetScaler
The scaler to be used for the inputs. If None, then no scaler is used
output_scaler: None of OffsetScaler
The scaler to be used for the outputs. If None, then no scaler is used
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Args:
lt_model: Linear-tree model
This is the Linear-tree model that will be loaded.
input_labels: list of str
The ordered list of labels corresponding to the inputs in the linear-tree model
output_labels: list of str
The ordered list of labels corresponding to the outputs in the linear-tree model
input_bounds: None of dict of tuples
Keys correspond to each of the input labels and values are the tuples of
bounds (lb, ub)
input_scaler: None or OffsetScaler
The scaler to be used for the inputs. If None, then no scaler is used
output_scaler: None of OffsetScaler
The scaler to be used for the outputs. If None, then no scaler is used
Args:
lt_model: Linear-tree model
This is the Linear-tree model that will be loaded.
input_labels: list of str
The ordered list of labels corresponding to the inputs in the linear-tree model.
output_labels: list of str
The ordered list of labels corresponding to the outputs in the linear-tree model.
input_bounds: None of dict of tuples
Keys correspond to each of the input labels and values are the tuples of
bounds (lb, ub).
input_scaler: None or OffsetScaler
The scaler to be used for the inputs. If None, then no scaler is used.
output_scaler: None of OffsetScaler
The scaler to be used for the outputs. If None, then no scaler is used.

@bpaul4
Copy link
Contributor

bpaul4 commented Sep 15, 2025

@bammari can you please look into the pylint issue? See below:

************* Module idaes.core.surrogate.linear_tree_surrogate
idaes/core/surrogate/linear_tree_surrogate.py:35:4: E0401: Unable to import 'omlt.linear_tree' (import-error)
idaes/core/surrogate/linear_tree_surrogate.py:35:4: E0611: No name 'linear_tree' in module 'omlt' (no-name-in-module)

Change to most recent version of omlt==1.2.2 to allow for linear-tree capabilities.
@bammari bammari requested a review from ksbeattie as a code owner September 15, 2025 21:15
@bammari
Copy link
Author

bammari commented Sep 15, 2025

@bpaul4 Thanks for pointing this out.

It looks like Github CI installs omlt v1.1 in the idaes-pse-dev conda environment when we run actions. Linear tree capabilities were introduced in omlt v1.2.

We have specified omlt==1.1 in setup.py. The most recent version is omlt==1.2.2. I believe changing this should resolve this issue. I have included a new commit that tests this.

bammari and others added 3 commits September 15, 2025 17:29
Add linear-tree as extra dependency.

Co-authored-by: Brandon Paul <[email protected]>
Change to linear-tree
Fix scikit-learn==1.6.1.
@ksbeattie ksbeattie requested a review from sufikaur as a code owner October 16, 2025 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority:Normal Normal Priority Issue or PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants