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.

Sign up for GitHub

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

Jump to bottom

docs: Data Preprocessor ADR #374

Merged
merged 2 commits into from
Nov 21, 2024
Merged

docs: Data Preprocessor ADR #374

merged 2 commits into from
Nov 21, 2024

Conversation

dushyantbehl
Copy link
Contributor

@dushyantbehl dushyantbehl commented Oct 17, 2024 •
edited
Loading

Description of the change

This PR adds an ADR for the newly proposed changes to the data pre procesor implementation in fms-hf-tuning.

@github-actions GitHub Actions
Copy link

Thanks for making a pull request! 😃
One of the maintainers will review and advise on the next steps.

@dushyantbehl dushyantbehl reopened this Oct 27, 2024
@willmj willmj changed the title Add first cut dataloader v2 ADR docs: Add first cut dataloader v2 ADR Oct 30, 2024
@github-actions github-actions bot added the docs label Oct 30, 2024
@willmj willmj mentioned this pull request Oct 30, 2024
Merged
2 tasks
@dushyantbehl dushyantbehl changed the title docs: Add first cut dataloader v2 ADR docs: Data Preprocessor v2 ADR Nov 5, 2024
@dushyantbehl dushyantbehl changed the title docs: Data Preprocessor v2 ADR ADR: Data Preprocessor Nov 5, 2024
@dushyantbehl dushyantbehl force-pushed the dataloader-v2 branch 3 times, most recently from 8710f68 to 1122244 Compare November 5, 2024 14:44
@dushyantbehl dushyantbehl marked this pull request as ready for review November 5, 2024 15:20
@dushyantbehl dushyantbehl changed the title ADR: Data Preprocessor docs: Data Preprocessor ADR Nov 5, 2024
@dushyantbehl dushyantbehl force-pushed the dataloader-v2 branch 2 times, most recently from 538479c to 8aef57e Compare November 18, 2024 16:59
willmj
willmj reviewed Nov 19, 2024
View reviewed changes
Copy link
Collaborator

@willmj willmj left a comment

Choose a reason for hiding this comment

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

This is very clear and gives great explanation of the use-cases, details, and design considerations. Have a couple of nitpicks and one question about required arguments below but it looks good to me!

architecture_records/004-datapreprocessor.md Outdated Show resolved Hide resolved
architecture_records/004-datapreprocessor.md Outdated Show resolved Hide resolved
architecture_records/004-datapreprocessor.md Outdated Show resolved Hide resolved
architecture_records/004-datapreprocessor.md Outdated Show resolved Hide resolved
architecture_records/004-datapreprocessor.md Outdated Show resolved Hide resolved
architecture_records/004-datapreprocessor.md Outdated Show resolved Hide resolved
architecture_records/004-datapreprocessor.md Outdated Show resolved Hide resolved
architecture_records/004-datapreprocessor.md Outdated Show resolved Hide resolved
@dushyantbehl
Copy link
Contributor Author

This is very clear and gives great explanation of the use-cases, details, and design considerations. Have a couple of nitpicks and one question about required arguments below but it looks good to me!

Thanks @willmj fixed the nits.

willmj
willmj approved these changes Nov 20, 2024
View reviewed changes
Copy link
Collaborator

@willmj willmj left a comment

Choose a reason for hiding this comment

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

Thanks Dushyant, looks good to me

Abhishek-TAMU
Abhishek-TAMU reviewed Nov 20, 2024
View reviewed changes
Copy link
Collaborator

@Abhishek-TAMU Abhishek-TAMU 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. Thanks Dushyant!

dushyantbehl and others added 2 commits November 21, 2024 09:22
@dushyantbehl
Add datapreprocessor v2 ADR
21ceb10 
Signed-off-by: Dushyant Behl <[email protected]>
@dushyantbehl @willmj
Update architecture_records/004-datapreprocessor.md
01fecad 
Co-authored-by: Will Johnson <[email protected]>
Signed-off-by: Dushyant Behl <[email protected]>
@ashokponkumar ashokponkumar merged commit a91a06d into foundation-model-stack:main Nov 21, 2024
8 checks passed
dushyantbehl added a commit to dushyantbehl/fms-hf-tuning that referenced this pull request Nov 22, 2024
@dushyantbehl
docs: Data Preprocessor ADR (foundation-model-stack#374)
60ac26c 
Signed-off-by: Dushyant Behl <[email protected]>
@dushyantbehl dushyantbehl deleted the dataloader-v2 branch December 3, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Abhishek-TAMU Abhishek-TAMU Abhishek-TAMU left review comments

@Ssukriti Ssukriti Ssukriti left review comments

@fabianlim fabianlim fabianlim left review comments

@ashokponkumar ashokponkumar ashokponkumar left review comments

@willmj willmj willmj approved these changes

@anhuong anhuong Awaiting requested review from anhuong anhuong is a code owner

@aluu317 aluu317 Awaiting requested review from aluu317 aluu317 is a code owner

@kmehant kmehant Awaiting requested review from kmehant kmehant is a code owner

Assignees
No one assigned
Labels
docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dushyantbehl @ashokponkumar @fabianlim @Ssukriti @willmj @Abhishek-TAMU