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

Updating the streaming version in setup.py #1103

Conversation

ShashankMosaicML
Copy link
Contributor

No description provided.

@ShashankMosaicML ShashankMosaicML marked this pull request as ready for review April 9, 2024 15:46
@ShashankMosaicML ShashankMosaicML requested a review from a team as a code owner April 9, 2024 15:46
@vchiley
Copy link
Contributor

vchiley commented Apr 9, 2024

Should we adde allow_unsafe_types=False, replication=None args here?

@ShashankMosaicML
Copy link
Contributor Author

allow_unsafe_types=False, replication=None

Done. PTAL.

Copy link
Contributor

@snarayan21 snarayan21 left a comment

Choose a reason for hiding this comment

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

Can you add the allow_unsafe_types and replication args as arguments to the StreamingTextDataset and StreamingFinetuningDataset init functions as well? What's the rationale for passing in fixed values? If I want to set replication from my yaml, for example, this isn't possible right

Copy link
Contributor

@vchiley vchiley left a comment

Choose a reason for hiding this comment

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

sorry nit

llmfoundry/data/finetuning/tasks.py Outdated Show resolved Hide resolved
llmfoundry/data/text_data.py Show resolved Hide resolved
@ShashankMosaicML
Copy link
Contributor Author

sorry nit

Done. Ptal.

@vchiley vchiley self-requested a review April 9, 2024 18:14
Copy link
Contributor

@karan6181 karan6181 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank You!

@ShashankMosaicML ShashankMosaicML merged commit 2939cc9 into mosaicml:main Apr 9, 2024
9 checks passed
KuuCi pushed a commit that referenced this pull request Apr 18, 2024
* updating the streaming version in setup.py

* updating constructor call to StreamingDataset

* making allow_unsafe_types and replication configurable through dataset configs

* adding docstring
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.

None yet

4 participants