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.

Sign up for GitHub

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

Jump to bottom

Remove BoTorch Workaround (old) #88

Closed
wants to merge 13 commits into from
Closed

Remove BoTorch Workaround (old) #88

wants to merge 13 commits into from

Conversation

Scienfitz
Copy link
Collaborator

@Scienfitz Scienfitz commented Jan 17, 2024 •
edited
Loading

  • removes the botorch workaround for hybrid recommender
  • bumps botorch version requirement
  • fix: Changelog had two changed sections
  • Python 3.8 can no longer be supported

I could not enable Python 3.12 support as several packages including torch do not support it yet, see pytorch/pytorch#110436

@Scienfitz Scienfitz added the enhancement Expand / change existing functionality label Jan 17, 2024
@Scienfitz Scienfitz self-assigned this Jan 17, 2024
@Scienfitz
Copy link
Collaborator Author

to merge this PR the requirements for expected action need to be changed:
image

but since other PRs depend on it I will only do that once all is approved

AdrianSosic
AdrianSosic requested changes Jan 18, 2024
View reviewed changes
Copy link
Collaborator

@AdrianSosic AdrianSosic left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, @Scienfitz! Here some things that require your attention. (BTW: also pushed one more commit removing some remaining 3.8 references)

CHANGELOG.md Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
baybe/recommenders/bayesian.py Show resolved Hide resolved
baybe/recommenders/bayesian.py Show resolved Hide resolved
@AdrianSosic AdrianSosic added the repo Requires changes to the project configuration label Jan 18, 2024
AdrianSosic
AdrianSosic approved these changes Jan 18, 2024
View reviewed changes
Copy link
Collaborator

@AdrianSosic AdrianSosic left a comment

Choose a reason for hiding this comment

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

Looking good 🚀

AVHopp
AVHopp approved these changes Jan 18, 2024
View reviewed changes
Copy link
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

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

Looks gucci to me :)

@Scienfitz Scienfitz force-pushed the feature/botorch_upgrade branch from 5ff61de to 8b606a0 Compare January 24, 2024 15:11
@Scienfitz Scienfitz changed the title Remove BoTorch Workaround Remove BoTorch Workaround (old) Feb 28, 2024
@Scienfitz Scienfitz mentioned this pull request Feb 28, 2024
Merged
@Scienfitz
Copy link
Collaborator Author

abandoned in favor of #151

@Scienfitz Scienfitz closed this Feb 28, 2024
Scienfitz added a commit that referenced this pull request Feb 28, 2024
@Scienfitz
Merge: Remove BoTorch Workaround (new) (#151)
b0d37f8 
- removes botorch workaround due to fixed type issue
- removes support for Python 3.8

reimplements #88
@Scienfitz Scienfitz deleted the feature/botorch_upgrade branch February 28, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdrianSosic AdrianSosic AdrianSosic approved these changes

@AVHopp AVHopp AVHopp approved these changes

Assignees

@Scienfitz Scienfitz

Labels
enhancement Expand / change existing functionality repo Requires changes to the project configuration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Scienfitz @AdrianSosic @AVHopp