haystack icon indicating copy to clipboard operation
haystack copied to clipboard

style: `max_new_tokens` are set twice with the same value

Open faaany opened this issue 2 years ago • 1 comments

Related Issues

In the following code snippet, we can see that for text-generation task, max_new_tokens are set twice: first set to self.max_length and then overwritten by max_length or self.max_length. But the first definition actually doesn't take any effect, because it will be overwritten by the second definition anyway.

            if is_text_generation and "return_full_text" not in model_input_kwargs:
                model_input_kwargs["return_full_text"] = False
                model_input_kwargs["max_new_tokens"] = self.max_length
            if stop_words:
                sw = StopWordsCriteria(tokenizer=self.pipe.tokenizer, stop_words=stop_words, device=self.pipe.device)
                model_input_kwargs["stopping_criteria"] = StoppingCriteriaList([sw])

            if "num_beams" in model_input_kwargs:
                num_beams = model_input_kwargs["num_beams"]
                if (
                    "num_return_sequences" in model_input_kwargs
                    and model_input_kwargs["num_return_sequences"] > num_beams
                ):
                    num_return_sequences = model_input_kwargs["num_return_sequences"]
                    logger.warning(
                        "num_return_sequences %s should not be larger than num_beams %s, hence setting it equal to num_beams",
                        num_return_sequences,
                        num_beams,
                    )
                    model_input_kwargs["num_return_sequences"] = num_beams

            # max_new_tokens is used for text-generation and max_length for text2text-generation
            if is_text_generation:
                model_input_kwargs["max_new_tokens"] = model_input_kwargs.pop("max_length", self.max_length)
            else:
                model_input_kwargs["max_length"] = self.max_length

Proposed Changes:

Remove the first one

How did you test it?

N/A

Notes for the reviewer

N/A

Checklist

faaany avatar Jul 14 '23 15:07 faaany

Pull Request Test Coverage Report for Build 5555588245

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.007%) to 45.481%

Files with Coverage Reduction New Missed Lines %
nodes/prompt/invocation_layer/hugging_face.py 1 92.19%
utils/context_matching.py 2 89.58%
<!-- Total: 3
Totals Coverage Status
Change from base Build 5545669749: -0.007%
Covered Lines: 10538
Relevant Lines: 23170

💛 - Coveralls

coveralls avatar Jul 14 '23 15:07 coveralls