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

[Forkless SparseML Transformers] [Feature Branch] Setting Up The modification module #2046

Closed
wants to merge 21 commits into from
Closed

[Forkless SparseML Transformers] [Feature Branch] Setting Up The modification module #2046

wants to merge 21 commits into from

Conversation

dbogunowicz
Copy link
Contributor

@dbogunowicz dbogunowicz commented Feb 8, 2024 •
edited
Loading

Summary

This pull request addresses the removal of the dependency on nm-transformers in favor of the original HF transformers. The primary motivation for this change is to simplify maintenance by eliminating the challenges associated with keeping a fork.

The scope of this PR was defined by running a diff between nm-transformers and transformers and identifying the elements that need to be included in sparseml to account for the differences.

For more details refer to the appropriate documentation in the internal docs repository.

Changes made:

  • adding a dependency on "transformers<4.37", removing the dependency on our fork
  • creating src/sparseml/transformers/sparsify/modification submodule, that is responsible for modifying the transformers models, as the fork did in the past
  • moving the appropriate scripts from sparseml.transformers.utils to sparseml.transformers.sparsification
  • updating the source code to support the new transformers version (e.g. accounting for renaming SiLUActivation to SiLU
  • adding exhaustive, new unit tests
  • fixed the issue of failing transformers test in GHA. Refactored and optimized the tests.

Manual Testing

Tested both Mistral and LlaMa on our end-to-end flows:

  1. Sparsification (one shot + sparse fine-tuning + quantization)
  2. Loading the sparsified model and inference
  3. Export of the sparsified model

Tested a single BERT QA recipe:

  1. Run the sparse fine-tuning
  2. Export the model

Note manual testing asserts that the pathways are working. The testing did not check the correctness of the resulting models.

Relevant PRs:

bfineran
bfineran reviewed Feb 26, 2024
View reviewed changes
src/sparseml/transformers/sparsification/modification/modify_model.py
:param model: The original HuggingFace transformers model
:return: The potentially modified model
"""
model_name = model.__class__.__name__
Copy link
Contributor

Choose a reason for hiding this comment

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

consider using the classes themselves as keys instead of name

src/sparseml/transformers/sparsification/modification/modify_model.py Outdated Show resolved Hide resolved
@dbogunowicz dbogunowicz changed the base branch from main to investigation/damian/open_files March 5, 2024 16:27
@dbogunowicz dbogunowicz changed the base branch from investigation/damian/open_files to main March 5, 2024 16:27
dbogunowicz and others added 15 commits March 11, 2024 13:06
@dbogunowicz
initial commit
c278130
@dbogunowicz
add omitted files
2114066
@dbogunowicz
more edits
8b51955
@dbogunowicz
[Forkless SparseML Transformers] Updating SparseML to be compatible w…
8bd369d 
…ith the forkless transformers (#2057)
@dbogunowicz
[Forkless SparseML Transformers] [Feature Branch] Modding Mistral (#…
80c185c 
…2047)

* working demo

* beautification

* [Forkless SparseML Transformers] [Feature Branch] Modding LLaMa (#2050)
@dbogunowicz
[Forkless SparseML Transformers] [Feature Branch] Modding OPT (#2051)
7f146b4 
* working demo

* beautification

* initial commit

* initial commit

* Fix typing mistakes

* [Forkless SparseML Transformers] [Feature Branch] Modding DistilBert(#2052)

* [Forkless SparseML Transformers] [Feature Branch] Modding `Bert` (#2054)

* initial commit

* initial commit

* [Forkless SparseML Transformers] [Feature Branch] Modding MobileBert (#2056)

* [Forkless SparseML Transformers] Hardening unit tests (#2088)

* initial commit

* bring back the original file

* initial commit
@dbogunowicz
consolidate tests
1717fa8
@dbogunowicz
small improvements according to the PR comments
a97a60f
@dbogunowicz
beautification
ac3d460
@dbogunowicz
fix failing export tests
8e40c20
pin pytest version to run tests in GHA
113d97f
fix tests and add modify_model functionality to non-llms models
5196960
swap recursion for iteration (swap_module function)
366ad56
working on transformers tests (fixing out of space error) 1
7afff08 
working on transformers tests (fixing out of space error) 2

working on transformers tests (fixing out of space error) 3

working on transformers tests (fixing out of space error) 4

fixing tests...

disable test_consecutive_runs

fix mistral

update integration tests
Add a checker for transformers version
fcb04a6
@dbogunowicz dbogunowicz force-pushed the feature/damian/forkless_transformer_feature branch from 9d47ad0 to fcb04a6 Compare March 11, 2024 12:08
dbogunowicz
dbogunowicz commented Mar 11, 2024
View reviewed changes
setup.py Outdated Show resolved Hide resolved
@dbogunowicz dbogunowicz requested a review from bfineran March 11, 2024 19:25
@dbogunowicz dbogunowicz mentioned this pull request Mar 28, 2024
Merged
@dbogunowicz
Copy link
Contributor Author

Closing, this PR is replaced by #2199 and #2204

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bfineran bfineran bfineran approved these changes

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