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

Jump to bottom

[FLAX] Question Answering Example #13649

Merged
merged 6 commits into from
Sep 21, 2021
Merged

[FLAX] Question Answering Example #13649

merged 6 commits into from
Sep 21, 2021

Conversation

kamalkraj
Copy link
Contributor

What does this PR do?

Flax Question Answering Example

Fixes # (issue)

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 who may be interested in your PR.

@patrickvonplaten @patil-suraj @sgugger

sgugger
sgugger approved these changes Sep 20, 2021
View reviewed changes
Copy link
Collaborator

@sgugger sgugger 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 adding that example!

examples/flax/question-answering/README.md
Comment on lines +35 to +43
```
huggingface-cli repo create bert-qa-squad-test
```

Next we clone the model repository to add the tokenizer and model files.

```
git clone https://huggingface.co/<your-username>/bert-qa-squad-test
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not linked to this PR per se, but this is not necessary while using the Repository API and all Flax examples should be updated to use it, while removing those instructions.

examples/flax/question-answering/README.md


### Usage notes
Note that when contexts are long they may be split into multiple training cases, not all of which may contain
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Note that when contexts are long they may be split into multiple training cases, not all of which may contain
Note that when contexts are long they may be split into multiple training cases, not all of which may contain

examples/flax/question-answering/utils_qa.py Show resolved Hide resolved
patil-suraj
patil-suraj approved these changes Sep 21, 2021
View reviewed changes
Copy link
Contributor

@patil-suraj patil-suraj left a comment

Choose a reason for hiding this comment

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

Looks really nice! Thanks for adding this example.

examples/flax/question-answering/run_qa.py Outdated Show resolved Hide resolved
examples/flax/question-answering/run_qa.py
Comment on lines +728 to +729
revision=model_args.model_revision,
use_auth_token=True if model_args.use_auth_token else None,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should pass dtype here for mixed-precision training, and seed for reproducibility.

For reference:

transformers/examples/flax/language-modeling/run_clm_flax.py

Lines 365 to 367 in 48fa42e

model = FlaxAutoModelForCausalLM.from_pretrained(
model_args.model_name_or_path, config=config, seed=training_args.seed, dtype=getattr(jnp, model_args.dtype)
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@patil-suraj
Why there is no Dynamic loss scaling for dtype float16 ?

https://flax.readthedocs.io/en/latest/_autosummary/flax.optim.DynamicScale.html#flax.optim.DynamicScale

cc @patrickvonplaten

Copy link
Contributor

Choose a reason for hiding this comment

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

After the #13098, dtype will specify only the dtype of computation, so just the forward pass will be in half-precision and grads will still be computed in fp32 so for this case loss scaling is not required.

True mixed-precision training for flax examples is planned and will include loss scaling, will post more about it soon :)

Also, note that we don't use flax optimizers here, we are using optax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the explanation

examples/flax/question-answering/run_qa.py Outdated Show resolved Hide resolved
@kamalkraj
Updates:
03a84d3 
1. Copyright Year updated
2. added dtype arg
3. passing seed and dtype to load model
4. Check eval flag before running eval
@kamalkraj
updated README
36b3c66
@kamalkraj
updated code comment
0d6e7c3
@kamalkraj
Copy link
Contributor Author

@patil-suraj
Done changes according to your review.

@patil-suraj patil-suraj merged commit 78807d8 into huggingface:master Sep 21, 2021
@kamalkraj kamalkraj deleted the flax-qa branch September 21, 2021 13:21
Albertobegue pushed a commit to Albertobegue/transformers that referenced this pull request Jan 13, 2022
@kamalkraj
[FLAX] Question Answering Example (huggingface#13649)
e5ce268 
* flax qa example

* Updated README:  Added Large model

* added utils_qa.py FULL_COPIES

* Updates:
1. Copyright Year updated
2. added dtype arg
3. passing seed and dtype to load model
4. Check eval flag before running eval

* updated README

* updated code comment
Albertobegue pushed a commit to Albertobegue/transformers that referenced this pull request Jan 27, 2022
@kamalkraj
[FLAX] Question Answering Example (huggingface#13649)
d902979 
* flax qa example

* Updated README:  Added Large model

* added utils_qa.py FULL_COPIES

* Updates:
1. Copyright Year updated
2. added dtype arg
3. passing seed and dtype to load model
4. Check eval flag before running eval

* updated README

* updated code comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patil-suraj patil-suraj patil-suraj approved these changes

@sgugger sgugger sgugger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kamalkraj @patil-suraj @sgugger