feat(causal message passing) by zachares · Pull Request #2 · nplan-io/transformers

zachares · 2023-05-09T13:58:11Z

causal message passing

vahanhov

Please check the comments before merging

vahanhov · 2023-05-11T17:44:10Z

src/transformers/models/bloom/causal_message_passing.py

+        if isinstance(succ_node, SequenceElement):
+            sequence_end_idx = succ_node.end_idx
+        elif isinstance(edge, SequenceElement):
+            sequence_end_idx = edge.end_idx
+        else:
+            sequence_end_idx = pred_node.end_idx
+        pred_node, edge, succ_node = edge_sequence[0]
+        if isinstance(succ_node, SequenceElement):
+            sequence_start_idx = succ_node.end_idx
+        elif isinstance(edge, SequenceElement):
+            sequence_start_idx = edge.end_idx
+        else:
+            sequence_start_idx = pred_node.end_idx


Suggested change

if isinstance(succ_node, SequenceElement):

sequence_end_idx = succ_node.end_idx

elif isinstance(edge, SequenceElement):

sequence_end_idx = edge.end_idx

else:

sequence_end_idx = pred_node.end_idx

pred_node, edge, succ_node = edge_sequence[0]

if isinstance(succ_node, SequenceElement):

sequence_start_idx = succ_node.end_idx

elif isinstance(edge, SequenceElement):

sequence_start_idx = edge.end_idx

else:

sequence_start_idx = pred_node.end_idx

if succ_node is not None:

sequence_end_idx = succ_node.end_idx

elif edge is not None:

sequence_end_idx = edge.end_idx

else:

sequence_end_idx = pred_node.end_idx

pred_node, edge, succ_node = edge_sequence[0]

if succ_node is not None:

sequence_start_idx = succ_node.end_idx

elif edge is not None:

sequence_start_idx = edge.end_idx

else:

sequence_start_idx = pred_node.end_idx

If this is correct, I find it more intuitive. Otherwise I had to go look into the code to answer the question of why are these checks mutually exclusive.

vahanhov · 2023-05-11T17:49:59Z

src/transformers/models/bloom/causal_message_passing.py

-        assert new_t_embeddings.shape == t_embeddings.shape
-        new_token_embeddings.append(new_t_embeddings.unsqueeze(0))
-    return torch.cat(new_token_embeddings, dim=0) + token_embeddings
+class CausalMessagePassingLayer(torch.nn.Module):


Eventually we should add a link to the paper here, because it's not at all trivial what's happening here

sounds good

vahanhov · 2023-05-11T17:53:53Z

src/transformers/models/bloom/modeling_bloom.py

@@ -853,25 +856,26 @@ def prepare_inputs_for_generation(
    ) -> dict:
        # only last token for input_ids if past is not None
        truncated_input_ids = input_ids


Suggested change

truncated_input_ids = input_ids

vahanhov · 2023-05-11T17:54:07Z

src/transformers/models/bloom/modeling_bloom.py

+        # past_key_values = None
+        if None:
            truncated_input_ids = input_ids[:, -1].unsqueeze(-1)

            # the cache may be in the stardard format (e.g. in contrastive search), convert to bloom's format if needed
            if past_key_values[0][0].shape[0] == input_ids.shape[0]:
                past_key_values = self._convert_to_bloom_cache(past_key_values)

        # if `inputs_embeds` are passed, we only want to use them in the 1st generation step


Suggested change

# past_key_values = None

if None:

truncated_input_ids = input_ids[:, -1].unsqueeze(-1)

# the cache may be in the stardard format (e.g. in contrastive search), convert to bloom's format if needed

if past_key_values[0][0].shape[0] == input_ids.shape[0]:

past_key_values = self._convert_to_bloom_cache(past_key_values)

# if `inputs_embeds` are passed, we only want to use them in the 1st generation step

vahanhov · 2023-05-11T17:54:54Z

src/transformers/models/bloom/modeling_bloom.py

+        # if inputs_embeds is not None and past_key_values is None:
+        #     model_inputs = {"inputs_embeds": inputs_embeds}
+        # else:
+        model_inputs = {"input_ids": truncated_input_ids}

        model_inputs.update(
            {


Suggested change

# if inputs_embeds is not None and past_key_values is None:

# model_inputs = {"inputs_embeds": inputs_embeds}

# else:

model_inputs = {"input_ids": truncated_input_ids}

model_inputs.update(

{

return {

vahanhov · 2023-05-11T17:56:06Z

src/transformers/models/bloom/modeling_bloom.py

+                # "full_input_ids": input_ids,
            }
        )
        return model_inputs


Suggested change

# "full_input_ids": input_ids,

}

)

return model_inputs

}

vahanhov · 2023-05-11T18:01:49Z

src/transformers/models/bloom/permutation_invariant_positions.py

+    return new_positions.unsqueeze(0)
+
+
+def _get_all_edge_previous_positions(


I didn't understand what's happening with these positions. Are you actually using this one?

yes I just debugged it yesterday, it sets all previous edges in a sequence to be directly before the current edge in terms of position.

src/transformers/models/bloom/modeling_bloom.py

* novelty debugging * running solution * message passing slightly better * simplified serialize * current code * flamingo inspired * message passing correctly implemented * positions update * removing commented code * causal message passing * edge case in case using another model besides serialize * update message passing and position embedding * Update src/transformers/models/bloom/modeling_bloom.py * removed unnecessary code

* init commit * config updated also some modeling * Processor and Model config combined * extraction pipeline(upto before spectogram & mel_conditioner) added but not properly tested * model loading successful! * feature extractor done! * FE can now be called from HF * postprocessing added in fe file * same as prev commit * Pop2PianoConfig doc done * cfg docs slightly changed * fe docs done * batched * batched working! * temp * v1 * checking * trying to go with generate * with generate and model tests passed * before rebasing * . * tests done docs done remaining others & nits * nits * LogMelSpectogram shifted to FeatureExtractor * is_tf rmeoved from pop2piano/init * import solved * tokenization tests added * minor fixed regarding modeling_pop2piano * tokenizer changed to only return midi_object and other changes * Updated paper abstract(Camera-ready version) (#2) * more comments and nits * ruff changes * code quality fix * sg comments * t5 change added and rebased * comments except batching * batching done * comments * small doc fix * example removed from modeling * ckpt * forward it compatible with fe and generation done * comments * comments * code-quality fix(maybe) * ckpts changed * doc file changed from mdx to md * test fixes * tokenizer test fix * changes * nits done main changes remaining * code modified * Pop2PianoProcessor added with tests * other comments * added Pop2PianoProcessor to dummy_objects * added require_onnx to modeling file * changes * update .md file * remove extra line in index.md * back to the main index * added pop2piano to index * Added tokenizer.__call__ with valid args and batch_decode and aligned the processor part too * changes * added return types to 2 tokenizer methods * the PR build test might work now * added backends * PR build fix * vocab added * comments * refactored vocab into 1 file * added conversion script * comments * essentia version changed in .md * comments * more tokenizer tests added * minor fix * tests extended for outputs acc check * small fix --------- Co-authored-by: Jongho Choi <sweetcocoa@snu.ac.kr>

…es (attempt #2) (huggingface#26784) * Update logits_process.py docstrings + match arg fields to __init__'s * Ran `make style`

zachares added 10 commits May 6, 2023 11:11

novelty debugging

2e4021b

running solution

aeb0cd5

message passing slightly better

928fba9

simplified serialize

25925e2

current code

b86771a

flamingo inspired

5b1cadf

message passing correctly implemented

01bf833

positions update

5a8fc12

removing commented code

a816c00

causal message passing

745963d

zachares requested a review from vahanhov May 9, 2023 14:51

zachares marked this pull request as ready for review May 9, 2023 14:52

edge case in case using another model besides serialize

db9d7f2

vahanhov approved these changes May 11, 2023

View reviewed changes

update message passing and position embedding

0bdcbd0

zachares commented May 12, 2023

View reviewed changes

src/transformers/models/bloom/modeling_bloom.py Outdated Show resolved Hide resolved

zachares and others added 3 commits May 12, 2023 09:48

Update src/transformers/models/bloom/modeling_bloom.py

35ad29f

removed unnecessary code

912975a

merge

bbcf0fb

zachares merged commit 844767b into main May 12, 2023

vahanhov deleted the novelty_debugging branch August 11, 2023 10:19

zachares pushed a commit that referenced this pull request Nov 20, 2023

Update logits_process.py docstrings to clarify penalty and reward cas…

0b8604d

…es (attempt #2) (huggingface#26784) * Update logits_process.py docstrings + match arg fields to __init__'s * Ran `make style`

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(causal message passing)#2

feat(causal message passing)#2
zachares merged 15 commits intomainfrom
novelty_debugging

zachares commented May 9, 2023

Uh oh!

vahanhov left a comment

Uh oh!

vahanhov May 11, 2023

Uh oh!

vahanhov May 11, 2023

Uh oh!

zachares May 12, 2023

Uh oh!

vahanhov May 11, 2023

Uh oh!

vahanhov May 11, 2023

Uh oh!

vahanhov May 11, 2023

Uh oh!

vahanhov May 11, 2023

Uh oh!

vahanhov May 11, 2023

Uh oh!

zachares May 12, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		return new_positions.unsqueeze(0)


		def _get_all_edge_previous_positions(

Conversation

zachares commented May 9, 2023

Uh oh!

vahanhov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants