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

Fix: n_task_params now evaluates to 1 if task_idx == 0 #187

Merged
merged 3 commits into from
Apr 3, 2024

Conversation

tobiasploetz
Copy link
Contributor

@tobiasploetz tobiasploetz commented Mar 28, 2024

fixes inconsistencies treating the TaskParameter that could cause errors when the other parts of the searchspace are purely continuous

@CLAassistant
Copy link

CLAassistant commented Mar 28, 2024

CLA assistant check
All committers have signed the CLA.

@AdrianSosic AdrianSosic added the bug Something isn't working label Apr 2, 2024
@AdrianSosic AdrianSosic self-assigned this Apr 2, 2024
@AdrianSosic AdrianSosic force-pushed the fix/tl-with-continuous-params branch from c54a02e to f494309 Compare April 2, 2024 08:09
@AdrianSosic AdrianSosic changed the title fix: n_task_params now evaluates to 1 if task_idx == 0 Fix: n_task_params now evaluates to 1 if task_idx == 0 Apr 2, 2024
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 @tobiasploetz for detecting and fixing the bug 👏🏼

@AVHopp
Copy link
Collaborator

AVHopp commented Apr 3, 2024

Unfortunately, this fix is not sufficient, there are some more issues regarding the use of transfer learning in "purely" continuous spaces (meaning all "real" parameters are continuous but the TaskParameter makes it hybrid).
I'll create an issue regarding this where I explain this in more detail and provide an example, I just wanted to already mention this here since this might or might not block this PR.

@AVHopp AVHopp force-pushed the fix/tl-with-continuous-params branch from 25045c2 to 4226ab4 Compare April 3, 2024 07:22
baybe/simulation.py Outdated Show resolved Hide resolved
baybe/simulation.py Outdated Show resolved Hide resolved
@AVHopp AVHopp force-pushed the fix/tl-with-continuous-params branch from 4226ab4 to f494309 Compare April 3, 2024 10:14
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.

Thanks for spotting and fixing @tobiasploetz :)

@AdrianSosic AdrianSosic force-pushed the fix/tl-with-continuous-params branch from f494309 to da191b0 Compare April 3, 2024 12:27
@AdrianSosic AdrianSosic merged commit 8a7f318 into main Apr 3, 2024
7 checks passed
@AdrianSosic AdrianSosic deleted the fix/tl-with-continuous-params branch April 3, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants