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: training #66

Merged
merged 2 commits into from
Jan 31, 2024
Merged

fix: training #66

merged 2 commits into from
Jan 31, 2024

Conversation

othertea
Copy link
Collaborator

@othertea othertea commented Jan 28, 2024

After the recently merged PRs, training with usual configs no longer worked. (I'm not sure how others have been running training scripts; I did not see any new training configs.)
This PR makes minimal changes so that we can once again train with the two provided configs: protein_lm/configs/train/toy_localcsv.yaml, protein_lm/configs/train/toy_hf.yaml.

@othertea othertea marked this pull request as ready for review January 28, 2024 01:34
@othertea othertea changed the title fix: fix training fix: training Jan 28, 2024
jamaliki

This comment was marked as duplicate.

@jamaliki jamaliki self-requested a review January 29, 2024 12:27
Copy link
Collaborator

@jamaliki jamaliki left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@NZ99 NZ99 left a comment

Choose a reason for hiding this comment

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

This looks good to me too -- thank you very much @othertea!

@othertea othertea merged commit 57a530d into OpenBioML:main Jan 31, 2024
@othertea othertea deleted the training/fix-training branch January 31, 2024 05:16
@talkhanz
Copy link
Contributor

Hey @othertea.

I'm running into issues when do_curriculum_learning is true.
The fields removed from protein_lm/configs/train/toy_localcsv.yaml and protein_lm/configs/train/toy_hfyaml as per this issue are required for curriculum learning to take place.

Can you let me know if you can reproduce an error when do_curriculum_learning is true in the config file?

@othertea
Copy link
Collaborator Author

Hi @talkhanz ,
Please feel free to create a new yaml file that does curriculum learning with the desired specifications! As their names indicate, the yamls in the repo are "toy" yamls that are used for sanity checking, not yamls intended to be used for the full training workflows. In particular, they are not guaranteed to work when you modify a single value, such as setting do_curriculum_learning to true.
This PR updated the yamls because the previous versions of the yamls did not run.

@talkhanz
Copy link
Contributor

makes sense. Thankyou @othertea!

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