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.

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

Fix typos in README and bugs in RAG example code for end-to-end evaluation and finetuning #9355

Merged

Conversation

yoshitomo-matsubara
Copy link
Contributor

What does this PR do?

This PR fixes bugs in RAG example code for end-to-end evaluation and finetuning.

1. Follow the file paths of reorganized examples

Also, the file paths for example code in README are updated (example/rag/ -> example/research_projects/rag/)

2. End-to-end evaluation

python examples/research_projects/rag/eval_rag.py \
    --model_name_or_path facebook/rag-sequence-nq \
    --model_type rag_sequence \
    --evaluation_set path/to/dev.source \
    --gold_data_path path/to/dev.gold_data \ # parsed `biencoder-nq-dev.json` following `qa` format
    --predictions_path path/to/e2e_preds.txt \
    --eval_mode e2e \
    --gold_data_mode qa \
    --n_docs 5 \ # You can experiment with retrieving different number of documents at evaluation time
    --print_predictions \
    --recalculate

With the above command, I encountered a few errors:

  1. an unexpected keyword argument 'clean_up_tokenization'
Some weights of RagSequenceForGeneration were not initialized from the model checkpoint at facebook/rag-sequence-nq and are newly initialized: ['rag.generator.lm_head.weight']
You should probably TRAIN this model on a down-stream task to be able to use it for predictions and inference.
initializing retrieval
Loading index from https://storage.googleapis.com/huggingface-nlp/datasets/wiki_dpr/
loading file https://storage.googleapis.com/huggingface-nlp/datasets/wiki_dpr/hf_bert_base.hnswSQ8_correct_phi_128.c_index.index.dpr from cache at /home/ubuntu/.cache/huggingface/transformers/a481b3aaed56325cb8901610e03e76f93b47f4284a1392d85e2ba5ce5d40d174.a382b038f1ea97c4fbad3098cd4a881a7cd4c5f73902c093e0c560511655cc0b
loading file https://storage.googleapis.com/huggingface-nlp/datasets/wiki_dpr/hf_bert_base.hnswSQ8_correct_phi_128.c_index.index_meta.dpr from cache at /home/ubuntu/.cache/huggingface/transformers/bb9560964463bc761c682818cbdb4e1662e91d25a9407afb102970f00445678c.f8cbe3240b82ffaad54506b5c13c63d26ff873d5cfabbc30eef9ad668264bab4
7it [00:00, 54.03it/s]
Traceback (most recent call last):
  File "examples/research_projects/rag/eval_rag.py", line 314, in <module>
    main(args)
  File "examples/research_projects/rag/eval_rag.py", line 300, in main
    answers = evaluate_batch_fn(args, model, questions)
  File "examples/research_projects/rag/eval_rag.py", line 134, in evaluate_batch_e2e
    print_docs=args.print_docs,
  File "/home/ubuntu/.local/share/virtualenvs/transformers-zPEj0XTF/lib/python3.7/site-packages/torch/autograd/grad_mode.py", line 26, in decorate_context
    return func(*args, **kwargs)
  File "/home/ubuntu/workspace/transformers/src/transformers/models/rag/modeling_rag.py", line 923, in generate
    **model_kwargs,
  File "/home/ubuntu/.local/share/virtualenvs/transformers-zPEj0XTF/lib/python3.7/site-packages/torch/autograd/grad_mode.py", line 26, in decorate_context
    return func(*args, **kwargs)
  File "/home/ubuntu/workspace/transformers/src/transformers/generation_utils.py", line 503, in generate
    model_kwargs = self._prepare_encoder_decoder_kwargs_for_generation(input_ids, model_kwargs)
  File "/home/ubuntu/workspace/transformers/src/transformers/generation_utils.py", line 86, in _prepare_encoder_decoder_kwargs_for_generation
    model_kwargs["encoder_outputs"]: ModelOutput = encoder(input_ids, return_dict=True, **encoder_kwargs)
  File "/home/ubuntu/.local/share/virtualenvs/transformers-zPEj0XTF/lib/python3.7/site-packages/torch/nn/modules/module.py", line 727, in _call_impl
    result = self.forward(*input, **kwargs)
TypeError: forward() got an unexpected keyword argument 'clean_up_tokenization'
  1. another unexpected keyword argument 'print_docs'
Some weights of RagSequenceForGeneration were not initialized from the model checkpoint at facebook/rag-sequence-nq and are newly initialized: ['rag.generator.lm_head.weight']
You should probably TRAIN this model on a down-stream task to be able to use it for predictions and inference.
initializing retrieval
Loading index from https://storage.googleapis.com/huggingface-nlp/datasets/wiki_dpr/
loading file https://storage.googleapis.com/huggingface-nlp/datasets/wiki_dpr/hf_bert_base.hnswSQ8_correct_phi_128.c_index.index.dpr from cache at /home/ubuntu/.cache/huggingface/transformers/a481b3aaed56325cb8901610e03e76f93b47f4284a1392d85e2ba5ce5d40d174.a382b038f1ea97c4fbad3098cd4a881a7cd4c5f73902c093e0c560511655cc0b
loading file https://storage.googleapis.com/huggingface-nlp/datasets/wiki_dpr/hf_bert_base.hnswSQ8_correct_phi_128.c_index.index_meta.dpr from cache at /home/ubuntu/.cache/huggingface/transformers/bb9560964463bc761c682818cbdb4e1662e91d25a9407afb102970f00445678c.f8cbe3240b82ffaad54506b5c13c63d26ff873d5cfabbc30eef9ad668264bab4
7it [00:00, 45.43it/s]
Traceback (most recent call last):
  File "examples/research_projects/rag/eval_rag.py", line 314, in <module>
    main(args)
  File "examples/research_projects/rag/eval_rag.py", line 300, in main
    answers = evaluate_batch_fn(args, model, questions)
  File "examples/research_projects/rag/eval_rag.py", line 134, in evaluate_batch_e2e
    print_docs=args.print_docs,
  File "/home/ubuntu/.local/share/virtualenvs/transformers-zPEj0XTF/lib/python3.7/site-packages/torch/autograd/grad_mode.py", line 26, in decorate_context
    return func(*args, **kwargs)
  File "/home/ubuntu/workspace/transformers/src/transformers/models/rag/modeling_rag.py", line 923, in generate
    **model_kwargs,
  File "/home/ubuntu/.local/share/virtualenvs/transformers-zPEj0XTF/lib/python3.7/site-packages/torch/autograd/grad_mode.py", line 26, in decorate_context
    return func(*args, **kwargs)
  File "/home/ubuntu/workspace/transformers/src/transformers/generation_utils.py", line 503, in generate
    model_kwargs = self._prepare_encoder_decoder_kwargs_for_generation(input_ids, model_kwargs)
  File "/home/ubuntu/workspace/transformers/src/transformers/generation_utils.py", line 86, in _prepare_encoder_decoder_kwargs_for_generation
    model_kwargs["encoder_outputs"]: ModelOutput = encoder(input_ids, return_dict=True, **encoder_kwargs)
  File "/home/ubuntu/.local/share/virtualenvs/transformers-zPEj0XTF/lib/python3.7/site-packages/torch/nn/modules/module.py", line 727, in _call_impl
    result = self.forward(*input, **kwargs)
TypeError: forward() got an unexpected keyword argument 'print_docs'

3. Finetuning

python examples/research_projects/rag/finetune_rag.py \
    --data_dir $DATA_DIR \
    --output_dir $OUTPUT_DIR \
    --model_name_or_path $MODEL_NAME_OR_PATH \
    --model_type rag_sequence \
    --fp16 \
    --gpus 8

With the above command, I found two easy bugs to be fixed:

  1. missing return parser returns None to parser and crashes here
  2. duplicated argument with num_retrieval_workers is also a blocker when using finetune_rag.py

Environments

  • Ubuntu 18.04 LTS
  • Python 3.7.7
  • transformers (I tried both 4.1.1 from pip and from repo 912f688)
  • torch: 1.7.1

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors which may be interested in your PR.

@patrickvonplaten @lhoestq


@staticmethod
def add_ray_specific_args(parser):
parser.add_argument(
Copy link
Contributor

Choose a reason for hiding this comment

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

why delete this argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As explained above, this argument is duplicate (See here) and causes an error

Copy link
Member

Choose a reason for hiding this comment

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

I would remove the other one and keep this one instead; sincice it's a ray-specific arg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the comment @lhoestq
I did so in the latest commit.

@@ -130,8 +130,6 @@ def evaluate_batch_e2e(args, rag_model, questions):
early_stopping=False,
num_return_sequences=1,
bad_words_ids=[[0, 0]], # BART likes to repeat BOS tokens, dont allow it to generate more than one
clean_up_tokenization=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

--src_path biencoder-nq-dev.json \
--evaluation_set output/biencoder-nq-dev.questions \
--gold_data_path output/biencoder-nq-dev.pages
```
3. Run evaluation:
```bash
python examples/rag/eval_rag.py \
python examples/research_projects/rag/eval_rag.py \
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Hey @yoshitomo-matsubara,

Thanks a lot for your PR! The changes look great to me - just one question, why can we delete the argument num_retrieval_workers?

@yoshitomo-matsubara
Copy link
Contributor Author

Hi @patrickvonplaten,

Thank you for reviewing this PR!
As commented above, the argument num_retrieval_workers in add_ray_specific_args is duplicate (first defined in add_retriever_specific_args) and causes an error.

Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Thanks for the very in-detail PR description @yoshitomo-matsubara

@patrickvonplaten
Copy link
Contributor

Great work @yoshitomo-matsubara

@patrickvonplaten patrickvonplaten merged commit d944966 into huggingface:master Jan 3, 2021
@yoshitomo-matsubara
Copy link
Contributor Author

Thank you for reviewing PR @patrickvonplaten @lhoestq !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants