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

[Export Refactor] Export transformers #1909

Merged
merged 7 commits into from
Dec 18, 2023
Merged

[Export Refactor] Export transformers #1909

merged 7 commits into from
Dec 18, 2023

Conversation

dbogunowicz
Copy link
Contributor

No description provided.

@dbogunowicz dbogunowicz requested a review from bfineran December 15, 2023 09:13
@dbogunowicz dbogunowicz force-pushed the feature/damian/export_transformers branch from 870a65b to 62c855b Compare December 15, 2023 09:21
@dbogunowicz dbogunowicz mentioned this pull request Dec 15, 2023
Merged
19 tasks
bfineran
bfineran reviewed Dec 15, 2023
View reviewed changes
src/sparseml/integration_helper_functions.py
Comment on lines +35 to +36
transformers = "transformers"
transformers_generative = "transformers-generative"
Copy link
Contributor

Choose a reason for hiding this comment

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

for top level integration - can we keep it as transformers and imply generative based on task name? this seems a bit cumbersome and hidden from UX perspective

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's discuss it today.

I agree that it would be more appropriate to "fold" the transformers_generative under transformers in this hierarchy of argument. The contrary,however, still feels like a good implementation for one important reason: in our context, transformers_generative are quite distinct from the rest of transformers. They have their own optimizations, potentially recipe loading logic (need to look into it), different set of files. There is also high probability that the more we focus on LLMs, and the more popularity they gain, over time they will more-and-more become a separate concept from the "normal" transformers.

This is why I feel that the separate IntegrationHelperFunctions for generative_transformers makes sense, and from that follows the separate integration name.

src/sparseml/integration_helper_functions.py
:return: The name of the integration to use for exporting the model.
"""

if integration is not None:
integration = integration.replace("_", "-")

if task is not None:
task = task.replace("_", "-")
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be resolved at a more consolidated level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like in the export.py?

src/sparseml/transformers/utils/initializers.py Outdated
_LOGGER = logging.getLogger(__name__)


# TODO: Let's not take this for granted. We should be able to
Copy link
Contributor

Choose a reason for hiding this comment

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

make sure to resolve all todos before merge

src/sparseml/transformers/utils/initializers.py Outdated
validation_dataset: Optional[Any] = None,
) -> Trainer:
"""
Initialize a trainer
Copy link
Contributor

Choose a reason for hiding this comment

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

doc string incomplete - should mention if this applies recipe structure, etc

additionally, what is the plan to migrate post sparseml v2 update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as mentioned in the follow-up PR, I will sync with @Satrat to learn about the most recent state of sparseml v2 and will update the logic. Let's not make it a blocker for landing this PR though.

@dbogunowicz dbogunowicz merged commit 450b286 into feature/damian/export_adapt Dec 18, 2023
@dbogunowicz dbogunowicz deleted the feature/damian/export_transformers branch December 18, 2023 15:40
dbogunowicz added a commit that referenced this pull request Dec 19, 2023
@dbogunowicz @bfineran
[Export Refactor] Prepare the module to be more general (before inclu…
506d7fe 
…ding `transformers`) (#1908)

* adapt the export script to handle transformers

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* Delete tests/sparseml/export/transformers/__init__.py

* Delete tests/sparseml/export/transformers/test_generative_transformers.py

* Delete tests/sparseml/export/transformers/test_transformers.py

* Update src/sparseml/export/export.py

Co-authored-by: Benjamin Fineran <[email protected]>

* addressing review comments

* [Export Refactor] Export `transformers` (#1909)

* cleanup

* Delete src/sparseml/transformers/integration_helper_functions_generative.py

* Delete src/sparseml/transformers/utils/optimizations.py

* Delete tests/sparseml/export/transformers/test_generative_transformers.py

* Delete tests/sparseml/transformers/test_integration_helper_functions_generative.py

* addressing PR reviews

* [Export Refactor] Export generative transformers(#1910)

* make tests green, remove using task to resolve the integration type

* fix all the tests after the merge, make integration resolution independent of the task name

* fold generative transformers into transformer helper functions

* complete tests for export_data.py

* Update src/sparseml/export/export.py

* add tests that confirms that kv cache injection has been added

* move applying optimizations into integration helper functions

---------

Co-authored-by: Benjamin Fineran <[email protected]>
dbogunowicz added a commit that referenced this pull request Dec 29, 2023
@dbogunowicz @bfineran
[Export Refactor] Prepare the module to be more general (before inclu…
3da5f23 
…ding `transformers`) (#1908)

* adapt the export script to handle transformers

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* Delete tests/sparseml/export/transformers/__init__.py

* Delete tests/sparseml/export/transformers/test_generative_transformers.py

* Delete tests/sparseml/export/transformers/test_transformers.py

* Update src/sparseml/export/export.py

Co-authored-by: Benjamin Fineran <[email protected]>

* addressing review comments

* [Export Refactor] Export `transformers` (#1909)

* cleanup

* Delete src/sparseml/transformers/integration_helper_functions_generative.py

* Delete src/sparseml/transformers/utils/optimizations.py

* Delete tests/sparseml/export/transformers/test_generative_transformers.py

* Delete tests/sparseml/transformers/test_integration_helper_functions_generative.py

* addressing PR reviews

* [Export Refactor] Export generative transformers(#1910)

* make tests green, remove using task to resolve the integration type

* fix all the tests after the merge, make integration resolution independent of the task name

* fold generative transformers into transformer helper functions

* complete tests for export_data.py

* Update src/sparseml/export/export.py

* add tests that confirms that kv cache injection has been added

* move applying optimizations into integration helper functions

---------

Co-authored-by: Benjamin Fineran <[email protected]>
bfineran added a commit that referenced this pull request Jan 10, 2024
@dbogunowicz @bfineran
[Export Refactor] Feature Branch (#1858)
691cb49 
* initial commit

* respond to PR comments

* [Export Refactor][Image Classification] `create_model` function (#1878)

* initial commit

* looking good, time to cleanup

* Delete src/sparseml/export/helpers.py

* Delete tests/sparseml/export/test_helpers.py

* ready for review

* improve design

* tests pass

* reuse _validate_dataset_num_classes

* [Export Refactor][Image Classification] `create_dummy_input` function (#1880)

* initial commit

* looking good, time to cleanup

* Delete src/sparseml/export/helpers.py

* Delete tests/sparseml/export/test_helpers.py

* ready for review

* improve design

* tests pass

* reuse _validate_dataset_num_classes

* initial commit

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* ready for review

* Update src/sparseml/export/export.py

* Update src/sparseml/integration_helper_functions.py

* [Export Refactor][Image Classification] `export_model` function (#1883)

* initial commit

* looking good, time to cleanup

* Delete src/sparseml/export/helpers.py

* Delete tests/sparseml/export/test_helpers.py

* ready for review

* improve design

* tests pass

* reuse _validate_dataset_num_classes

* initial commit

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* ready for review

* Update src/sparseml/export/export.py

* Update src/sparseml/integration_helper_functions.py

* initial commit

* fixes

* ready for review

* nit

* add return

* make export function more general

* [Export Refactor][Image Classification] `apply_optimizations` function (#1884)

* initial commit

* looking good, time to cleanup

* Delete src/sparseml/export/helpers.py

* Delete tests/sparseml/export/test_helpers.py

* ready for review

* improve design

* tests pass

* reuse _validate_dataset_num_classes

* initial commit

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* ready for review

* Update src/sparseml/export/export.py

* Update src/sparseml/integration_helper_functions.py

* initial commit

* fixes

* ready for review

* nit

* add return

* initial commit

* [Export Refactor][Image Classification] `export_sample_inputs_outputs` function (#1888)

* initial commit

* looking good, time to cleanup

* Delete src/sparseml/export/helpers.py

* Delete tests/sparseml/export/test_helpers.py

* ready for review

* improve design

* tests pass

* reuse _validate_dataset_num_classes

* initial commit

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* ready for review

* Update src/sparseml/export/export.py

* Update src/sparseml/integration_helper_functions.py

* initial commit

* fixes

* ready for review

* nit

* add return

* initial commit

* initial commit

* PR comments

* beautification

* remove duplicated function

* [Export Refactor][Image Classification] `create_deployment_folder` function (#1889)

* initial commit

* looking good, time to cleanup

* Delete src/sparseml/export/helpers.py

* Delete tests/sparseml/export/test_helpers.py

* ready for review

* improve design

* tests pass

* reuse _validate_dataset_num_classes

* initial commit

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* ready for review

* Update src/sparseml/export/export.py

* Update src/sparseml/integration_helper_functions.py

* initial commit

* fixes

* ready for review

* nit

* add return

* initial commit

* initial commit

* initial commit

* fix rebase, tests_work

* ready to push

* [Export Refactor][Image Classification] `validate_correctness` function (#1890)

* initial commit

* looking good, time to cleanup

* Delete src/sparseml/export/helpers.py

* Delete tests/sparseml/export/test_helpers.py

* ready for review

* improve design

* tests pass

* reuse _validate_dataset_num_classes

* initial commit

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* ready for review

* Update src/sparseml/export/export.py

* Update src/sparseml/integration_helper_functions.py

* initial commit

* fixes

* ready for review

* nit

* add return

* initial commit

* initial commit

* initial commit

* initial commit

* Delete tests/sparseml/test_integration_helper_functions.py

* ready to merge

* [Export Refactor] End to end testing (#1898)

* initial commit

* looking good, time to cleanup

* Delete src/sparseml/export/helpers.py

* Delete tests/sparseml/export/test_helpers.py

* ready for review

* improve design

* tests pass

* reuse _validate_dataset_num_classes

* initial commit

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* ready for review

* Update src/sparseml/export/export.py

* Update src/sparseml/integration_helper_functions.py

* initial commit

* fixes

* ready for review

* nit

* add return

* initial commit

* initial commit

* initial commit

* initial commit

* Delete tests/sparseml/test_integration_helper_functions.py

* ready to merge

* add structure validator

* ready for review

* Delete tests/sparseml/export/model.onnx

* Delete tests/sparseml/export/image_classification/model.onnx

* Delete tests/sparseml/export/image_classification/conftest.py

* PR comments

* remove onnx

* [Export Refactor] Prepare the module to be more general (before including `transformers`) (#1908)

* adapt the export script to handle transformers

* Update src/sparseml/pytorch/image_classification/integration_helper_functions.py

* Delete tests/sparseml/export/transformers/__init__.py

* Delete tests/sparseml/export/transformers/test_generative_transformers.py

* Delete tests/sparseml/export/transformers/test_transformers.py

* Update src/sparseml/export/export.py

Co-authored-by: Benjamin Fineran <[email protected]>

* addressing review comments

* [Export Refactor] Export `transformers` (#1909)

* cleanup

* Delete src/sparseml/transformers/integration_helper_functions_generative.py

* Delete src/sparseml/transformers/utils/optimizations.py

* Delete tests/sparseml/export/transformers/test_generative_transformers.py

* Delete tests/sparseml/transformers/test_integration_helper_functions_generative.py

* addressing PR reviews

* [Export Refactor] Export generative transformers(#1910)

* make tests green, remove using task to resolve the integration type

* fix all the tests after the merge, make integration resolution independent of the task name

* fold generative transformers into transformer helper functions

* complete tests for export_data.py

* Update src/sparseml/export/export.py

* add tests that confirms that kv cache injection has been added

* move applying optimizations into integration helper functions

---------

Co-authored-by: Benjamin Fineran <[email protected]>

* [Export Refactor][Transformers] Enable loading SparseModels (#1921)

* initial commit

* adressing review comments

* Fix the tests

* fix tests with help from sara

* [Export][Transformers] Enable loading `text-generation` datasets (#1938)

* add suport for past_key_values in sample-outputs

* [Export][Transformers] Implementation of correctness validation (#1935)

* fix tests with help from sara

* Update src/sparseml/transformers/utils/initializers.py

* swap sparsezoo validator for custom one (top k match)

* add more informative error message

* add correctness validation for LLMs

* remove past_key_values from outputs

* remove past_key_values from outputs (2)

* small note comment for the future

* tests fixed

* fix test

* [Export refactor] final manual testing fixes (#1948)

* [Export refactor] final manual testing fixes

* review

---------

Co-authored-by: Benjamin Fineran <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bfineran bfineran bfineran left review comments

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

Successfully merging this pull request may close these issues.
2 participants
@dbogunowicz @bfineran