TFEncoderDecoderModel generate() gvies different results after #15562

ydshieh commented 2 years ago

Environment info

transformers version: 4.17.0.dev0
Platform: Linux-5.13.0-1015-gcp-x86_64-with-glibc2.31
Python version: 3.9.7
PyTorch version (GPU?): 1.10.2+cu102 (False)
Tensorflow version (GPU?): 2.7.0 (False)
Flax version (CPU?/GPU?/TPU?): 0.4.0 (cpu)
Jax version: 0.3.1
JaxLib version: 0.3.0
Using GPU in script?: No
Using distributed or parallel set-up in script?: No

Information

TFEncoderDecoderModel.generate() for ydshieh/bert2bert-cnn_dailymail-fp16 gives different results after #15562, see below.

To reproduce

PT

article = """(CNN)Sigma Alpha Epsilon is under fire for a video showing party-bound fraternity members singing a racist chant. SAE's national chapter suspended the students, but University of Oklahoma President David Boren took it a step further, saying the university's affiliation with the fraternity is permanently done. The news is shocking, but it's not the first time SAE has faced controversy. SAE was founded March 9, 1856, at the University of Alabama, five years before the American Civil War, according to the fraternity website. When the war began, the group had fewer than 400 members, of which "369 went to war for the Confederate States and seven for the Union Army," the website says. The fraternity now boasts more than 200,000 living alumni, along with about 15,000 undergraduates populating 219 chapters and 20 "colonies" seeking full membership at universities. SAE has had to work hard to change recently after a string of member deaths, many blamed on the hazing of new recruits, SAE national President Bradley Cohen wrote in a message on the fraternity's website. The fraternity's website lists more than 130 chapters cited or suspended for "health and safety incidents" since 2010. At least 30 of the incidents involved hazing, and dozens more involved alcohol. However, the list is missing numerous incidents from recent months. Among them, according to various media outlets: Yale University banned the SAEs from campus activities last month after members allegedly tried to interfere with a sexual misconduct investigation connected to an initiation rite. Stanford University in December suspended SAE housing privileges after finding sorority members attending a fraternity function were subjected to graphic sexual content. And Johns Hopkins University in November suspended the fraternity for underage drinking. "The media has labeled us as the 'nation's deadliest fraternity,' " Cohen said. In 2011, for example, a student died while being coerced into excessive alcohol consumption, according to a lawsuit. SAE's previous insurer dumped the fraternity. "As a result, we are paying Lloyd's of London the highest insurance rates in the Greek-letter world," Cohen said. Universities have turned down SAE's attempts to open new chapters, and the fraternity had to close 12 in 18 months over hazing incidents."""
expected = """sae was founded in 1856, five years before the civil war. the fraternity has had to work hard to change recently. the university of oklahoma president says the university's affiliation with the fraternity is permanently done. the sae has had a string of members in recent months."""

from transformers import AutoTokenizer, EncoderDecoderModel

loc = "patrickvonplaten/bert2bert-cnn_dailymail-fp16"

model = EncoderDecoderModel.from_pretrained(loc)
tokenizer = AutoTokenizer.from_pretrained(loc)
input_ids = tokenizer(article, return_tensors="pt").input_ids
output_ids = model.generate(input_ids, use_cache=False)
summary = tokenizer.decode(output_ids[0], skip_special_tokens=True)
print(summary)
print(summary == expected)

On both commit a3dbbc346 and 2e12b907a, this gives sae was founded in 1856, ...

TF


article = """(CNN)Sigma Alpha Epsilon is under fire for a video showing party-bound fraternity members singing a racist chant. SAE's national chapter suspended the students, but University of Oklahoma President David Boren took it a step further, saying the university's affiliation with the fraternity is permanently done. The news is shocking, but it's not the first time SAE has faced controversy. SAE was founded March 9, 1856, at the University of Alabama, five years before the American Civil War, according to the fraternity website. When the war began, the group had fewer than 400 members, of which "369 went to war for the Confederate States and seven for the Union Army," the website says. The fraternity now boasts more than 200,000 living alumni, along with about 15,000 undergraduates populating 219 chapters and 20 "colonies" seeking full membership at universities. SAE has had to work hard to change recently after a string of member deaths, many blamed on the hazing of new recruits, SAE national President Bradley Cohen wrote in a message on the fraternity's website. The fraternity's website lists more than 130 chapters cited or suspended for "health and safety incidents" since 2010. At least 30 of the incidents involved hazing, and dozens more involved alcohol. However, the list is missing numerous incidents from recent months. Among them, according to various media outlets: Yale University banned the SAEs from campus activities last month after members allegedly tried to interfere with a sexual misconduct investigation connected to an initiation rite. Stanford University in December suspended SAE housing privileges after finding sorority members attending a fraternity function were subjected to graphic sexual content. And Johns Hopkins University in November suspended the fraternity for underage drinking. "The media has labeled us as the 'nation's deadliest fraternity,' " Cohen said. In 2011, for example, a student died while being coerced into excessive alcohol consumption, according to a lawsuit. SAE's previous insurer dumped the fraternity. "As a result, we are paying Lloyd's of London the highest insurance rates in the Greek-letter world," Cohen said. Universities have turned down SAE's attempts to open new chapters, and the fraternity had to close 12 in 18 months over hazing incidents."""
expected = """sae was founded in 1856, five years before the civil war. the fraternity has had to work hard to change recently. the university of oklahoma president says the university's affiliation with the fraternity is permanently done. the sae has had a string of members in recent months."""

from transformers import AutoTokenizer, TFEncoderDecoderModel

loc = "ydshieh/bert2bert-cnn_dailymail-fp16"

model = TFEncoderDecoderModel.from_pretrained(loc)
tokenizer = AutoTokenizer.from_pretrained(loc)
input_ids = tokenizer(article, return_tensors="tf").input_ids
output_ids = model.generate(input_ids, use_cache=False)
summary = tokenizer.decode(output_ids[0], skip_special_tokens=True)
print(summary)
print(summary == expected)

commit a3dbbc346 : sae was founded in 1856, (same as the PyTorch version)
commit 2e12b907a : sae's national chapter suspended students, ...

Who can help

@patrickvonplaten (generate)

patrickvonplaten commented 2 years ago

@ydshieh - thanks for the PR!

Tiny feedback, when we format code you use:

class Example:

instead of

class Example:

it's a bit easier to quickly read the code this way.

ydshieh commented 2 years ago

@patrickvonplaten I will take a look of this issue if you haven't been able to find the time on it.

I think it would be great if we can fix this before the next release which will have exciting news about TF generate :-).

patrickvonplaten commented 2 years ago

This would be amazing if you find a bit of time for it @ydshieh

ydshieh commented 2 years ago

Fixed by #17426 by the changes in generation_tf_utils.py

        is_pad_token_not_equal_to_eos_token_id = (eos_token_id is None) or (
            (eos_token_id is not None) and (pad_token_id != eos_token_id)
        )

huggingface / transformers