P-tuning refactor Part 3/N #6106

arendu · 2023-02-24T06:57:02Z

What does this PR do ?

Add a one line overview of what this PR aims to accomplish.

Collection: [Note which collection this PR will affect]

Changelog

Add specific line by line info of high level changes in this PR.

Usage

You can potentially add a usage example below

# Add a code snippet demonstrating how to use this

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

Related to # (issue)

… attribute Signed-off-by: arendu <[email protected]>

Signed-off-by: arendu <[email protected]>

for more information, see https://pre-commit.ci

Signed-off-by: arendu <[email protected]>

for more information, see https://pre-commit.ci

Signed-off-by: arendu <[email protected]>

…NVIDIA/NeMo into adithyare/refac_ptuning_part3.2

Signed-off-by: arendu <[email protected]>

for more information, see https://pre-commit.ci

Signed-off-by: arendu <[email protected]>

…NVIDIA/NeMo into adithyare/refac_ptuning_part3.2

for more information, see https://pre-commit.ci

yidong72 · 2023-02-24T20:18:43Z

nemo/collections/nlp/models/dialogue/dialogue_gpt_classification_model.py

@@ -291,7 +291,7 @@ def get_prompt_token_labels_for_megatron_gpt(self, input_ids, num_prompt_tokens)

    def get_virtual_prompt_ids_for_megatron_gpt(self, input_ids):
        if (
-            self.cfg.virtual_prompt_style == VirtualPromptStyle.PROMPT_TUNING
+            self.cfg.virtual_prompt_style == VirtualPromptStyle.P_TUNING


VirtualPromptStyle.PROMPT_TUNING is removed from the enum?

okay. saw it. NVM

yidong72 · 2023-02-24T20:20:00Z

nemo/collections/nlp/models/language_modeling/megatron_base_prompt_learning_model.py

            total_virtual_tokens=total_virtual_tokens,
            token_dim=self.hidden_size,
-            hidden_size=self.cfg.p_tuning.get("encoder_hidden", 2048),
+            hidden_size=self.cfg.p_tuning.get("encoder_hidden", self.hidden_size // 2),


is self.hidden_size // 2 a good number to use?

It's a better default, for when the base model has different hidden sizes.

yidong72 · 2023-02-24T20:23:22Z

nemo/collections/nlp/models/language_modeling/megatron_gpt_prompt_learning_model.py

@@ -62,7 +64,7 @@
 __all__ = ['MegatronGPTPromptLearningModel']


-class MegatronGPTPromptLearningModel(MegatronBaseModel, TextGeneration):
+class MegatronGPTPromptLearningModel(MegatronBasePromptLearningModel):


why drop the TextGeneration interface?

The base model containers it now!

The base class now has TextGenetation.

Zhilin123

LGTM, Great to see a lot of redundant code removed from this PR. Minor stylistic changes, but ok to merge, if you can address them in the next PR

Zhilin123 · 2023-02-24T20:25:02Z

nemo/collections/nlp/models/language_modeling/megatron_base_prompt_learning_model.py

@@ -59,7 +58,9 @@ class MegatronBasePromptLearningModel(MegatronBaseModel, TextGeneration):

    def __init__(self, cfg: DictConfig, trainer: Trainer):
        super().__init__(cfg, trainer)
+        self.init_model(cfg, trainer)


I see here (in the lines above) that you have imports you don't use - maybe remove them in the refactor 4/n ? vscode should be able to identify them

There's quite a few in other files as well

Agreed! will do!

Zhilin123 · 2023-02-24T20:26:36Z

nemo/collections/nlp/models/language_modeling/megatron_base_prompt_learning_model.py

-            raise ValueError(
-                f"\nvirtual prompt style '{cfg.virtual_prompt_style}' not recognized, please use one of 'prompt-tuning' or 'p-tuning'"
-            )
+            raise ValueError(f"\nvirtual prompt style '{cfg.virtual_prompt_style}'")


Please indicate what they should use instead. e.g please use p-tuning

Zhilin123 · 2023-02-24T20:27:05Z

nemo/collections/nlp/models/language_modeling/megatron_base_prompt_learning_model.py

@@ -211,68 +170,25 @@ def init_prompt_encoder(self):
        new_task = self.new_tasks[0]
        total_virtual_tokens = self.task_templates[new_task]["total_virtual_tokens"]

-        encoder_type = PromptEncoderType(self.cfg.p_tuning.get("encoder_type", "mlp").lower())
+        encoder_type = PromptEncoderType(self.cfg.p_tuning.get("encoder_type", "tpmlp").lower())


Is there a reason why default was changed?

No reason, it was during my copy based from the child model.

Zhilin123 · 2023-02-24T20:28:25Z

nemo/collections/nlp/models/language_modeling/megatron_base_prompt_learning_model.py

-                self.prompt_encoder.load_state_dict(state_dict_, strict)
-
-    def embed_input_train(self, input_ids: Tensor, taskname_ids: Tensor):
+                raise ValueError("invalid virtual prompt source")


please indicate what is the correct config. E.g. please set this cfg.... to p_tuning

* patch to allow using tokenizers without additional_special_tokens_ids attribute Signed-off-by: arendu <[email protected]> * steps to inheret gpt from base Signed-off-by: arendu <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * steps to inheret gpt from base Signed-off-by: arendu <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix in pred step Signed-off-by: arendu <[email protected]> * moved to base model Signed-off-by: arendu <[email protected]> * removing prompt table class moved into prompt encoder class Signed-off-by: arendu <[email protected]> * minor fix Signed-off-by: arendu <[email protected]> * minor fix Signed-off-by: arendu <[email protected]> * changes in dialogue models Signed-off-by: arendu <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * minor updates to classifications Signed-off-by: arendu <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Signed-off-by: arendu <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* patch to allow using tokenizers without additional_special_tokens_ids attribute Signed-off-by: arendu <[email protected]> * steps to inheret gpt from base Signed-off-by: arendu <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * steps to inheret gpt from base Signed-off-by: arendu <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix in pred step Signed-off-by: arendu <[email protected]> * moved to base model Signed-off-by: arendu <[email protected]> * removing prompt table class moved into prompt encoder class Signed-off-by: arendu <[email protected]> * minor fix Signed-off-by: arendu <[email protected]> * minor fix Signed-off-by: arendu <[email protected]> * changes in dialogue models Signed-off-by: arendu <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * minor updates to classifications Signed-off-by: arendu <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Signed-off-by: arendu <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Signed-off-by: hsiehjackson <[email protected]>

arendu added 29 commits December 15, 2022 10:51

patch to allow using tokenizers without additional_special_tokens_ids…

2b95406

… attribute Signed-off-by: arendu <[email protected]>

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

c131a90

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

9e15c3a

merge main

d0e3669

Signed-off-by: arendu <[email protected]>

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

0a19a5a

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

ec3d57b

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

64e36ba

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

5bfde7e

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

b04b145

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

b1906ab

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

9795062

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

0f83085

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

ee4dd1a

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

53ba0b2

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

a6aee2a

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

33442d4

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

8e6c5c9

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

efd263c

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

ecfda4f

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

15aee0c

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

2b7f3de

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

f62cde9

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

31915c9

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

fa22a1f

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

e62cd47

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

321a907

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

aeaf13f

Merge branch 'main' of https://github.com/NVIDIA/NeMo into main

c9a61f1

steps to inheret gpt from base

252c5ed

Signed-off-by: arendu <[email protected]>

github-actions bot added the NLP label Feb 24, 2023

pre-commit-ci bot and others added 4 commits February 24, 2023 06:57

[pre-commit.ci] auto fixes from pre-commit.com hooks

8a7e23c

for more information, see https://pre-commit.ci

steps to inheret gpt from base

50bcfb5

Signed-off-by: arendu <[email protected]>

steps to inheret gpt from base

7c0cb77

Signed-off-by: arendu <[email protected]>

[pre-commit.ci] auto fixes from pre-commit.com hooks

1808262

for more information, see https://pre-commit.ci

arendu changed the title ~~Adithyare/refac ptuning part3.2~~ Prompt tuning refactor Part 3/N Feb 24, 2023

arendu changed the title ~~Prompt tuning refactor Part 3/N~~ P-tuning refactor Part 3/N Feb 24, 2023

arendu and others added 13 commits February 24, 2023 05:27

fix in pred step

bc25a8f

Signed-off-by: arendu <[email protected]>

Merge branch 'adithyare/refac_ptuning_part3.2' of https://github.com/…

963538f

…NVIDIA/NeMo into adithyare/refac_ptuning_part3.2

moved to base model

6cae9d8

Signed-off-by: arendu <[email protected]>

removing prompt table class moved into prompt encoder class

ca5bc2c

Signed-off-by: arendu <[email protected]>

minor fix

204a3d0

Signed-off-by: arendu <[email protected]>

minor fix

5c2734a

Signed-off-by: arendu <[email protected]>

changes in dialogue models

c21814c

Signed-off-by: arendu <[email protected]>

Merge branch 'main' into adithyare/refac_ptuning_part3.2

e82bf3c

[pre-commit.ci] auto fixes from pre-commit.com hooks

69158c3

for more information, see https://pre-commit.ci

minor updates to classifications

5e16431

Signed-off-by: arendu <[email protected]>

Merge branch 'adithyare/refac_ptuning_part3.2' of https://github.com/…

9b834b1

…NVIDIA/NeMo into adithyare/refac_ptuning_part3.2

[pre-commit.ci] auto fixes from pre-commit.com hooks

58c70a4

for more information, see https://pre-commit.ci

Merge branch 'main' into adithyare/refac_ptuning_part3.2

86312d7

arendu requested review from Zhilin123, yidong72 and MaximumEntropy February 24, 2023 19:09

yidong72 reviewed Feb 24, 2023

View reviewed changes

Zhilin123 approved these changes Feb 24, 2023

View reviewed changes

arendu merged commit dabd8b8 into main Feb 24, 2023

arendu deleted the adithyare/refac_ptuning_part3.2 branch February 24, 2023 21:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

P-tuning refactor Part 3/N #6106

P-tuning refactor Part 3/N #6106

arendu commented Feb 24, 2023

yidong72 Feb 24, 2023

yidong72 Feb 24, 2023

yidong72 Feb 24, 2023

arendu Feb 24, 2023

yidong72 Feb 24, 2023

arendu Feb 24, 2023

arendu Feb 24, 2023

Zhilin123 left a comment

Zhilin123 Feb 24, 2023

Zhilin123 Feb 24, 2023

arendu Feb 24, 2023

Zhilin123 Feb 24, 2023

Zhilin123 Feb 24, 2023

arendu Feb 24, 2023

Zhilin123 Feb 24, 2023

P-tuning refactor Part 3/N #6106

P-tuning refactor Part 3/N #6106

Conversation

arendu commented Feb 24, 2023

What does this PR do ?

Changelog

Usage

Before your PR is "Ready for review"

Who can review?

Additional Information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zhilin123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment