Skip to content
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

Linear Tree Implementation #108

Merged
merged 105 commits into from
Sep 18, 2023

Conversation

bammari
Copy link
Collaborator

@bammari bammari commented May 19, 2023

This PR now allows users to embed linear model decision trees (trained with the linear-tree package https://github.com/cerlymarco/linear-tree) within OMLT. We include several univariate and bivariate tests to ensure the several implemented formulations are functional. Furthermore, this PR includes a preliminary Jupyter notebook that shows how to embed these linear model decision trees. using OMLT

In addition to this implementation, this PR also rewrites the way scaled_input_bounds are defined within the _setup_scaled_inputs_and_outputs function in formulation.py. Specifically, we pass a dictionary of tuples containing lower and upper bounds rather than constructing a bounds rule. test_maxpool_FullSpaceNNFormulation was failing without this change.

Legal Acknowledgement
By contributing to this software project, I agree my contributions are submitted under the BSD license.
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.

.github/workflows/main.yml Show resolved Hide resolved
docs/notebooks/linear_trees/linear_tree_formulations.ipynb Outdated Show resolved Hide resolved
src/omlt/linear_tree/lt_definition.py Show resolved Hide resolved
src/omlt/linear_tree/lt_definition.py Show resolved Hide resolved
src/omlt/linear_tree/lt_definition.py Show resolved Hide resolved
src/omlt/linear_tree/lt_definition.py Show resolved Hide resolved
tests/linear_tree/test_lt_formulation.py Outdated Show resolved Hide resolved
tests/linear_tree/test_lt_formulation.py Outdated Show resolved Hide resolved
@rmisener rmisener self-requested a review August 22, 2023 13:31
Copy link
Member

@rmisener rmisener left a comment

Choose a reason for hiding this comment

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

Just noticed something small while working through some material, this should be easy.

Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to arrange the placement of the notebooks so that bo_with_trees.ipynb is in the same folder as this new notebook? At least it's confusing to me why the notebooks for the trees are in separate places. If linear trees are so different from the GBT implementations that they do need to be in a different folder, perhaps add one more folder so that it's notebooks/trees and then have everything inside?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Per your suggestion, I consolidated both of these notebooks in notebooks/trees.

In addition, I updated docs/notebooks.rst.

@rmisener
Copy link
Member

Please also add a description of the new Jupyter notebook in docs/notebooks.rst

@rmisener
Copy link
Member

Going through minor things and want to make sure @bammari gets credit for this work. Would you please also add to this PR:

In README.rst

  • Please add a reference to the Computers & Chemical Engineering paper. State that if anyone uses the linear tree implementation, they should cite the C&CE paper.
  • Please add @bammari to the list of contributors at the bottom and state his funding.

In the new notebook, please add the C&CE paper as a reference. As an example of how to do this, see bo_with_trees.ipynb

@bammari
Copy link
Collaborator Author

bammari commented Sep 11, 2023

Going through minor things and want to make sure @bammari gets credit for this work. Would you please also add to this PR:

In README.rst

  • Please add a reference to the Computers & Chemical Engineering paper. State that if anyone uses the linear tree implementation, they should cite the C&CE paper.
  • Please add @bammari to the list of contributors at the bottom and state his funding.

In the new notebook, please add the C&CE paper as a reference. As an example of how to do this, see bo_with_trees.ipynb

Added reference to CACE paper in both the notebook and Readme.rst. I also updated the OMLT paper reference from Arxiv to Journal of ML Research in README.rst. In addition, added contribution statement - Bashar

@carldlaird carldlaird merged commit b60bf0d into cog-imperial:main Sep 18, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants