support value attribution condition #6934

yidong72 · 2023-06-28T01:58:15Z

support value attribute string prediction
support value attribute sft
update UI to allow user enter values.

Signed-off-by: Yi Dong <[email protected]>

MaximumEntropy

Few minor comments.

MaximumEntropy · 2023-06-28T02:49:30Z

nemo/collections/nlp/data/language_modeling/megatron/gpt_sft_chat_dataset.py

    cur_idx = header_len
    tgt_len = target.shape[0]
    for i, (tokenized_len, speaker, s_id) in enumerate(zip(tokenized_lens, speakers, s_ids)):
        # note, sentence piece will add extra empty token in front. s_id has that extra token too
        skip_name_len = len(tokenizer.text_to_ids(TURN_TOKEN + speaker + END_NAME_SIGNAL))
+        if (s_id[1:] == 255002).any().item():


Can we check detokenized string value instead of a hardcode value specific to a particular tokenizer?

done. but it is still associated with sentence piece as this tokenizer added an extra token in front. Need to test other tokenizers.

Okay. I fixed the code a bit and tested on the huggingface tokenizer and confirm it is working.

MaximumEntropy · 2023-06-28T02:57:36Z

examples/nlp/language_modeling/conf/megatron_gpt_inference.yaml

+chat: False # use the chat interface
+chatbot_config:
+  value: False   # whether to inject the value attributes


Can you call this steerlm_config instead?

the paper is in anonymity. don't want to mention the name of the method.

MaximumEntropy · 2023-06-28T02:58:12Z

nemo/collections/nlp/data/language_modeling/megatron/gpt_sft_chat_dataset.py

@@ -26,21 +26,34 @@
 END_SIGNAL = "\n"
 END_NAME_SIGNAL = "\n"

+SCALE = 9


Can we avoid making this a global variable?

It makes it hard to experimentat with different kinds of value models.

removed this. only use the value string

Signed-off-by: Yi Dong <[email protected]>

Zhilin123

Some comments on readability

Zhilin123 · 2023-06-28T17:25:28Z

nemo/collections/nlp/data/language_modeling/megatron/gpt_sft_chat_dataset.py

+    if isinstance(label, str):
+        return '<extra_id_2>' + label + '\n'
+    else:
+        raise ValueError(f'Unknown label type {type(label)}')


Maybe more informative error msg (e.g. please have label in str format instead)

nemo/collections/nlp/data/language_modeling/megatron/gpt_sft_chat_dataset.py

Zhilin123 · 2023-06-28T17:31:22Z

nemo/collections/nlp/data/language_modeling/megatron/gpt_sft_chat_dataset.py

@@ -157,14 +196,21 @@ def _build_samples_mapping(self):
        assert hasattr(self.tokenizer, "vocab"), "tokenizer should have vocab property, not supported"
        assert '<extra_id_0>' in self.tokenizer.vocab, "<extra_id_0> not in the tokenizer vocab. not supported"
        assert '<extra_id_1>' in self.tokenizer.vocab, "<extra_id_1> not in the tokenizer vocab. not supported"
+        # calcuilate <extra_id_2> id value
+        if '<extra_id_2>' in self.tokenizer.vocab:
+            ids_1 = self.tokenizer.text_to_ids('<extra_id_1><extra_id_2>')


is this a typo?

Not sure why we can't get the text_to_ids('<extra_id_2>') directly. This looks pretty hacky

this is to handle the sentencepiece tokenizer which adds a special token in front. A hacky way I agree.

Btw this check is super slow since self.tokenizer.vocab is a list not a dictionary/set. It is actually faster to do len(self.tokenizer.text_to_ids('<extra_id_2>')) == 1.

Hmm, nit but isn't it just gonna be self.extra_id_2_token_id = self.tokenizer.text_to_ids('<extra_id_2>')[-1]

nemo/collections/nlp/data/language_modeling/megatron/gpt_sft_chat_dataset.py

Zhilin123 · 2023-06-28T17:48:22Z

nemo/collections/nlp/data/language_modeling/megatron/gpt_sft_chat_dataset.py

 }


-def _mask_targets(target, tokenized_lens, speakers, header_len, s_ids, tokenizer, mask_role):
+def _mask_targets(target, tokenized_lens, speakers, header_len, s_ids, tokenizer, mask_role, gtype, extra_id_2_id):


Can you write a description on what the code is doing using an example such as (also include the control tokens)

e.g.

(user utterance1) (bot utterance1 ) (user utterance2) (bot utterance)
XXXXXXXXXX YYYY ZZZZZZZZZZZZ AAAA → loss on YYYY and AAAA together

as well as what each arg does?

This function is pretty hard to read without it.

Signed-off-by: Yi Dong <[email protected]>

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

Signed-off-by: Yi Dong <[email protected]>

Zhilin123

Generally looks good, with some remaining minor code style issues but we can address in future PR since code freeze is today.

Signed-off-by: Yi Dong <[email protected]>

yidong72 added 13 commits May 19, 2023 01:35

text gen condition on value

b6fd0bc

Signed-off-by: Yi Dong <[email protected]>

fix round function

90f9896

Signed-off-by: Yi Dong <[email protected]>

predict value

2a6d169

Signed-off-by: Yi Dong <[email protected]>

scale 9

eb2e824

Signed-off-by: Yi Dong <[email protected]>

handle hard code label

1a9dc88

Signed-off-by: Yi Dong <[email protected]>

use likert scale 7

271d8fc

Signed-off-by: Yi Dong <[email protected]>

scale 6

4b417b2

Signed-off-by: Yi Dong <[email protected]>

Merge branch 'main' into value_ds

412fdf7

Merge branch 'main' into value_ds

b320575

Merge branch 'main' into value_ds

67d1305

merge the latest main

8c0968d

Signed-off-by: Yi Dong <[email protected]>

added latest chatbot ui

49448a6

Signed-off-by: Yi Dong <[email protected]>

added new playground interface

3a28c78

Signed-off-by: Yi Dong <[email protected]>

yidong72 requested a review from MaximumEntropy June 28, 2023 01:58

github-actions bot added the NLP label Jun 28, 2023

yidong72 requested a review from ericharper June 28, 2023 01:58

default scale 9

6cce277

Signed-off-by: Yi Dong <[email protected]>

MaximumEntropy suggested changes Jun 28, 2023

View reviewed changes

yidong72 and others added 2 commits June 28, 2023 14:33

address comments

bf46e8d

Signed-off-by: Yi Dong <[email protected]>

Merge branch 'main' into value_ds

7b706f2

Zhilin123 requested changes Jun 28, 2023

View reviewed changes

yidong72 and others added 6 commits June 28, 2023 19:51

add speicial tokens

eacf248

Signed-off-by: Yi Dong <[email protected]>

handles more tokenizer

d5ea1f7

Signed-off-by: Yi Dong <[email protected]>

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

7cc4c81

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

added comments

aad6866

Signed-off-by: Yi Dong <[email protected]>

added comments

ea270be

Signed-off-by: Yi Dong <[email protected]>

Merge branch 'main' into value_ds

f04895e

yidong72 requested review from Zhilin123 and MaximumEntropy June 28, 2023 21:41

fix type

c8b7d49

Signed-off-by: Yi Dong <[email protected]>

Zhilin123 previously approved these changes Jun 28, 2023

View reviewed changes

faster check

33922c6

Signed-off-by: Yi Dong <[email protected]>

yidong72 dismissed Zhilin123’s stale review via 33922c6 June 28, 2023 22:01

MaximumEntropy approved these changes Jun 28, 2023

View reviewed changes

yidong72 merged commit a27ba52 into main Jun 29, 2023
15 checks passed

yidong72 deleted the value_ds branch June 29, 2023 02:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support value attribution condition #6934

support value attribution condition #6934

yidong72 commented Jun 28, 2023

MaximumEntropy left a comment

MaximumEntropy Jun 28, 2023

yidong72 Jun 28, 2023

yidong72 Jun 28, 2023

MaximumEntropy Jun 28, 2023

yidong72 Jun 28, 2023

MaximumEntropy Jun 28, 2023

MaximumEntropy Jun 28, 2023

yidong72 Jun 28, 2023

Zhilin123 left a comment

Zhilin123 Jun 28, 2023

yidong72 Jun 28, 2023

Zhilin123 Jun 28, 2023

Zhilin123 Jun 28, 2023

yidong72 Jun 28, 2023

MaximumEntropy Jun 28, 2023

Zhilin123 Jun 28, 2023 •

edited

Loading

Zhilin123 Jun 28, 2023

yidong72 Jun 28, 2023

Zhilin123 left a comment

support value attribution condition #6934

support value attribution condition #6934

Conversation

yidong72 commented Jun 28, 2023

MaximumEntropy 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

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

Zhilin123 Jun 28, 2023 • edited Loading

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

Zhilin123 Jun 28, 2023 •

edited

Loading