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 bugs in Optuna integration with the prompt2model demo script #374

Merged
merged 2 commits into from
Oct 31, 2023

Conversation

viswavi
Copy link
Collaborator

@viswavi viswavi commented Oct 30, 2023

Description

#315 introduced automated hyperparameter selection in Prompt2Model via Optuna, but this PR did not work with the demo script out-of-the-box. This PR fixes those bugs.

References

N/A

Blocked by

N/A

Copy link
Collaborator

@neubig neubig left a comment

Choose a reason for hiding this comment

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

Two questions:

  1. Do we need to fix the jupyter notebook too?
  2. Is there a way we can add tests to catch when the demo scripts are broken? One idea is to make sure that we have tests that basically replicate the code in the demo scripts.

Copy link
Collaborator

@zhaochenyang20 zhaochenyang20 left a comment

Choose a reason for hiding this comment

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

The code itself looks good to me.

@viswavi
Copy link
Collaborator Author

viswavi commented Oct 31, 2023

@neubig The Jupyter notebook was not affected by the earlier changes - right now the notebook does not run hyperparameter selection. I will add this in a subsequent PR, but want to fix this (broken) capability in the demo script first.

Regarding the integration tests, I agree that we need this somewhat urgently. I've filed an issue for this and will tackle this next week:
#375

@viswavi viswavi merged commit 13a2049 into main Oct 31, 2023
8 checks passed
@viswavi viswavi deleted the vijay_fix_bugs_in_optuna_integration branch October 31, 2023 16:03
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.

3 participants