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

Shorten the conversation tests for speed + fixing position overflows #26960

Merged
merged 11 commits into from
Oct 31, 2023
Merged

Shorten the conversation tests for speed + fixing position overflows #26960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
42c4d25
Shorten the conversation tests for speed + fixing position overflows
Rocketknight1 Oct 20, 2023
f44f2af
Put max_new_tokens back to 5
Rocketknight1 Oct 23, 2023
131754f
Remove test skips
Rocketknight1 Oct 23, 2023
6922575
Increase max_position_embeddings in blenderbot tests
Rocketknight1 Oct 23, 2023
23cddb1
Add skips for blenderbot_small
Rocketknight1 Oct 23, 2023
1d6f7b6
Correct TF test skip
Rocketknight1 Oct 23, 2023
d4eaeea
make fixup
Rocketknight1 Oct 23, 2023
bfd2813
Reformat skips to use is_pipeline_test_to_skip
Rocketknight1 Oct 27, 2023
0421bb9
Update tests/models/blenderbot_small/test_modeling_blenderbot_small.py
Rocketknight1 Oct 31, 2023
5b19e9c
Update tests/models/blenderbot_small/test_modeling_flax_blenderbot_sm…
Rocketknight1 Oct 31, 2023
6d5a2b4
Update tests/models/blenderbot_small/test_modeling_tf_blenderbot_smal…
Rocketknight1 Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/models/blenderbot/test_modeling_blenderbot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def __init__(
hidden_act="gelu",
hidden_dropout_prob=0.1,
attention_probs_dropout_prob=0.1,
max_position_embeddings=20,
max_position_embeddings=50,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change won't be reflect on the tiny models uploaded to Hub (hf-internal-testing). So the pipeline testing will still use the previous value. We still skip the test, so it is fine, but the change here has no effect.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(the above is just a comment, not a request to change)

eos_token_id=2,
pad_token_id=1,
bos_token_id=0,
Expand Down
2 changes: 1 addition & 1 deletion tests/models/blenderbot/test_modeling_flax_blenderbot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def __init__(
hidden_act="gelu",
hidden_dropout_prob=0.1,
attention_probs_dropout_prob=0.1,
max_position_embeddings=32,
max_position_embeddings=50,
eos_token_id=2,
pad_token_id=1,
bos_token_id=0,
Expand Down
2 changes: 1 addition & 1 deletion tests/models/blenderbot/test_modeling_tf_blenderbot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def __init__(
intermediate_size=37,
hidden_dropout_prob=0.1,
attention_probs_dropout_prob=0.1,
max_position_embeddings=20,
max_position_embeddings=50,
eos_token_id=2,
pad_token_id=1,
bos_token_id=0,
Expand Down
9 changes: 2 additions & 7 deletions tests/models/blenderbot_small/test_modeling_blenderbot_small.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def __init__(
hidden_act="gelu",
hidden_dropout_prob=0.1,
attention_probs_dropout_prob=0.1,
max_position_embeddings=20,
max_position_embeddings=50,
eos_token_id=2,
pad_token_id=1,
bos_token_id=0,
Expand Down Expand Up @@ -242,12 +242,7 @@ class BlenderbotSmallModelTest(ModelTesterMixin, GenerationTesterMixin, Pipeline
def is_pipeline_test_to_skip(
self, pipeline_test_casse_name, config_class, model_architecture, tokenizer_name, processor_name
):
if pipeline_test_casse_name == "TextGenerationPipelineTests":
return True
# TODO @Rocketnight1 to fix
if pipeline_test_casse_name == "ConversationalPipelineTests":
return True
return False
return pipeline_test_casse_name in ("TextGenerationPipelineTests", "ConversationalPipelineTests")

def setUp(self):
self.model_tester = BlenderbotSmallModelTester(self)
Expand Down
7 changes: 6 additions & 1 deletion tests/models/blenderbot_small/test_modeling_flax_blenderbot_small.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def __init__(
hidden_act="gelu",
hidden_dropout_prob=0.1,
attention_probs_dropout_prob=0.1,
max_position_embeddings=32,
max_position_embeddings=50,
eos_token_id=2,
pad_token_id=1,
bos_token_id=0,
Expand Down Expand Up @@ -320,6 +320,11 @@ class FlaxBlenderbotSmallModelTest(FlaxModelTesterMixin, unittest.TestCase, Flax
)
all_generative_model_classes = (FlaxBlenderbotSmallForConditionalGeneration,) if is_flax_available() else ()

def is_pipeline_test_to_skip(
self, pipeline_test_casse_name, config_class, model_architecture, tokenizer_name, processor_name
):
return pipeline_test_casse_name in ("TextGenerationPipelineTests", "ConversationalPipelineTests")

def setUp(self):
self.model_tester = FlaxBlenderbotSmallModelTester(self)

Expand Down
16 changes: 6 additions & 10 deletions tests/models/blenderbot_small/test_modeling_tf_blenderbot_small.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def __init__(
intermediate_size=37,
hidden_dropout_prob=0.1,
attention_probs_dropout_prob=0.1,
max_position_embeddings=20,
max_position_embeddings=50,
eos_token_id=2,
pad_token_id=1,
bos_token_id=0,
Expand Down Expand Up @@ -198,6 +198,11 @@ class TFBlenderbotSmallModelTest(TFModelTesterMixin, PipelineTesterMixin, unitte
test_pruning = False
test_onnx = False

def is_pipeline_test_to_skip(
self, pipeline_test_casse_name, config_class, model_architecture, tokenizer_name, processor_name
):
return pipeline_test_casse_name in ("TextGenerationPipelineTests", "ConversationalPipelineTests")

def setUp(self):
self.model_tester = TFBlenderbotSmallModelTester(self)
self.config_tester = ConfigTester(self, config_class=BlenderbotSmallConfig)
Expand All @@ -209,15 +214,6 @@ def test_decoder_model_past_large_inputs(self):
config_and_inputs = self.model_tester.prepare_config_and_inputs_for_common()
self.model_tester.check_decoder_model_past_large_inputs(*config_and_inputs)

# TODO: Fix the failed tests when this model gets more usage
def is_pipeline_test_to_skip(
self, pipeline_test_casse_name, config_class, model_architecture, tokenizer_name, processor_name
):
# TODO @Rocketnight1 to fix
if pipeline_test_casse_name == "ConversationalPipelineTests":
return True
return False


@require_tokenizers
@require_tf
Expand Down
8 changes: 4 additions & 4 deletions tests/pipelines/test_pipelines_conversational.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,14 @@ def get_test_pipeline(self, model, tokenizer, processor):

def run_pipeline_test(self, conversation_agent, _):
# Simple
outputs = conversation_agent(Conversation("Hi there!"), max_new_tokens=20)
outputs = conversation_agent(Conversation("Hi there!"), max_new_tokens=5)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ok

self.assertEqual(
outputs,
Conversation([{"role": "user", "content": "Hi there!"}, {"role": "assistant", "content": ANY(str)}]),
)

# Single list
outputs = conversation_agent([Conversation("Hi there!")], max_new_tokens=20)
outputs = conversation_agent([Conversation("Hi there!")], max_new_tokens=5)
self.assertEqual(
outputs,
Conversation([{"role": "user", "content": "Hi there!"}, {"role": "assistant", "content": ANY(str)}]),
Expand All @@ -96,7 +96,7 @@ def run_pipeline_test(self, conversation_agent, _):
self.assertEqual(len(conversation_1), 1)
self.assertEqual(len(conversation_2), 1)

outputs = conversation_agent([conversation_1, conversation_2], max_new_tokens=20)
outputs = conversation_agent([conversation_1, conversation_2], max_new_tokens=5)
self.assertEqual(outputs, [conversation_1, conversation_2])
self.assertEqual(
outputs,
Expand All @@ -118,7 +118,7 @@ def run_pipeline_test(self, conversation_agent, _):

# One conversation with history
conversation_2.add_message({"role": "user", "content": "Why do you recommend it?"})
outputs = conversation_agent(conversation_2, max_new_tokens=20)
outputs = conversation_agent(conversation_2, max_new_tokens=5)
self.assertEqual(outputs, conversation_2)
self.assertEqual(
outputs,
Expand Down
2 changes: 1 addition & 1 deletion tests/test_pipeline_mixin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ def data(n):

out = []
if task == "conversational":
for item in pipeline(data(10), batch_size=4, max_new_tokens=20):
for item in pipeline(data(10), batch_size=4, max_new_tokens=5):
out.append(item)
else:
for item in pipeline(data(10), batch_size=4):
Expand Down