Skip to content

Quantization fixes#8701

Merged
ericharper merged 9 commits intomainfrom
jlasek/quantization_fixes
Mar 20, 2024
Merged

Quantization fixes#8701
ericharper merged 9 commits intomainfrom
jlasek/quantization_fixes

Conversation

@janekl
Copy link
Collaborator

@janekl janekl commented Mar 19, 2024

What does this PR do ?

Fixing quantization issues based on more extensive tests:

  • model configured with megatron_amp_O2: true
  • model w/o artifacts
  • model using HF tokenizer
  • remove broken "pileval" dataset

Collection: NLP

Changelog

See above.

Jenkins CI

To run Jenkins, a NeMo User with write access must comment jenkins on the PR.

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation?
  • Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
    • Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

  • New Feature
  • Bugfix
  • Documentation

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

janekl added 2 commits March 19, 2024 13:48
Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
@janekl janekl force-pushed the jlasek/quantization_fixes branch from 2da19b8 to 21b21ee Compare March 19, 2024 15:09
@janekl janekl requested review from dimapihtar and ericharper March 19, 2024 15:11
suiyoubi
suiyoubi previously approved these changes Mar 19, 2024
model_file = app_state.model_restore_path
model_cfg = copy.deepcopy(model.cfg)
if not hasattr(model, "artifacts"):
OmegaConf.save(model_cfg.tokenizer, os.path.join(output_dir, "tokenizer_config.yaml"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

there's no guarantee that model_cfg has an attribute tokenizer

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks - addressed here 8adc16d.

Do you have an example LLM w/o tokenizer? I'll put together a test for this.

@ericharper
Copy link
Collaborator

jenkins

janekl added 4 commits March 20, 2024 13:41
Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
@janekl
Copy link
Collaborator Author

janekl commented Mar 20, 2024

jenkins

@janekl janekl requested a review from ericharper March 20, 2024 13:00
janekl added 2 commits March 20, 2024 14:03
Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
@janekl
Copy link
Collaborator Author

janekl commented Mar 20, 2024

jenkins

Copy link
Collaborator

@ericharper ericharper left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@ericharper ericharper merged commit 5b5a444 into main Mar 20, 2024
@ericharper ericharper deleted the jlasek/quantization_fixes branch March 20, 2024 16:52
rohitrango pushed a commit to rohitrango/NeMo that referenced this pull request Jun 25, 2024
* Handle model w/o artifacts

Signed-off-by: Jan Lasek <janek.lasek@gmail.com>

* Quantization for megatron_amp_O2=True case and test for it

Signed-off-by: Jan Lasek <janek.lasek@gmail.com>

* Remove broken pileval dataset

Signed-off-by: Jan Lasek <janek.lasek@gmail.com>

* Consider model w/o tokenizer case

Signed-off-by: Jan Lasek <janek.lasek@gmail.com>

* Use default converter precision

Signed-off-by: Jan Lasek <janek.lasek@gmail.com>

* Code formatting

Signed-off-by: Jan Lasek <janek.lasek@gmail.com>

* Fix typo in Jenkinsfile

Signed-off-by: Jan Lasek <janek.lasek@gmail.com>

* Disable sequence parallel for evaluation

Signed-off-by: Jan Lasek <janek.lasek@gmail.com>

---------

Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments