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

Add EM Generalist Finetuning #241

Closed
wants to merge 17 commits into from
Closed

Add EM Generalist Finetuning #241

wants to merge 17 commits into from

Conversation

anwai98
Copy link
Contributor

@anwai98 anwai98 commented Oct 24, 2023

  • The training scripts for lm generalist datasets (still need to test it and add necessary docstrings)
  • TODO: need to make sure the data exists before checking RoIs (for getting data splits), else the script will crash

@anwai98 anwai98 marked this pull request as ready for review October 25, 2023 10:51
@anwai98
Copy link
Contributor Author

anwai98 commented Oct 25, 2023

This is good to go. I tested the training for a few epochs, looks fine. Thanks!

Copy link
Contributor

@constantinpape constantinpape left a comment

Choose a reason for hiding this comment

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

Found one typo and am a bit confused why the LM files are still in this PR.

@@ -0,0 +1,116 @@
import os
Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused why these files are part of this PR, they should have been merged via #228 already!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I might know the reason. Probably because I created a branch from lm-generalist branch, hence it also took those new files into consideration (which at that moment weren't a part of master)

@@ -0,0 +1,97 @@
import os
Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused why these files are part of this PR, they should have been merged via #228 already!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

@anwai98 anwai98 closed this Oct 25, 2023
@anwai98 anwai98 deleted the em-generalist branch May 5, 2024 23:37
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