[WIP] refactor namespaces #1714

erip · 2020-02-16T20:40:42Z

Before submitting

Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
Did you read the contributor guideline?
Did you make sure to update the docs?
Did you write any new necessary tests?

What does this PR do?

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

…s as an attribute.

… interface.

…new criterion interface.

…args as an attribute.

…interface.

…rface.

…lf.lr with a double.

…s as an attribute.

…tribute. TODO: make a base class?

… an attribute. TODO: make a base class?

myleott · 2020-02-18T15:09:56Z

This is looking pretty good so far, thanks!

One comment: I don't think we should do the full refactoring in a single PR. Let's have one for optimizers, one for tasks, one for models, etc.

The reason is that before we can merge, we need to import the PR and run internal unit/integration tests, which usually requires some additional changes on the FB side. If we can keep these more focused it will make that part easier for me :)

erip · 2020-02-18T16:19:07Z

Ok, can do. I'll try to cherry-pick these into a separate branch tonight.

myleott · 2020-02-22T00:41:21Z

Awesome, I'll start running these against the internal unit tests :)

erip · 2020-02-24T23:28:48Z

@myleott looks like they're all good - anything you need from me?

erip · 2020-03-02T21:35:42Z

@myleott sorry to keep harassing you - since these files tend to be hot, I wanted to see if there's anything you need from me in order to merge them so we can prevent the merge-conflict=>reimport loop. 😄

Summary: # Before submitting - [x] Was this discussed/approved via a Github issue? (no need for typos, doc improvements) - [x] Did you read the [contributor guideline](https://github.com/pytorch/fairseq/blob/master/CONTRIBUTING.md)? - [x] Did you make sure to update the docs? - [x] Did you write any new necessary tests? ## What does this PR do? Fixes facebookresearch/fairseq#1672 in part (part 1: [context](facebookresearch/fairseq#1714 (comment))) ## PR review Anyone in the community is free to review the PR once the tests have passed. If we didn't discuss your PR in Github issues there's a high chance it will not be merged. ## Did you have fun? Make sure you had fun coding � Pull Request resolved: facebookresearch/fairseq#1729 Differential Revision: D20049353 Pulled By: myleott fbshipit-source-id: 732077a1cc339c9f7ebe26dae42a7e8d7b5a07b4

Summary: # Before submitting - [x] Was this discussed/approved via a Github issue? (no need for typos, doc improvements) - [x] Did you read the [contributor guideline](https://github.com/pytorch/fairseq/blob/master/CONTRIBUTING.md)? - [x] Did you make sure to update the docs? - [x] Did you write any new necessary tests? ## What does this PR do? Fixes #1672 in part (part 1: [context](#1714 (comment))) ## PR review Anyone in the community is free to review the PR once the tests have passed. If we didn't discuss your PR in Github issues there's a high chance it will not be merged. ## Did you have fun? Make sure you had fun coding � Pull Request resolved: #1729 Differential Revision: D20049353 Pulled By: myleott fbshipit-source-id: 732077a1cc339c9f7ebe26dae42a7e8d7b5a07b4

Summary: # Before submitting - [x] Was this discussed/approved via a Github issue? (no need for typos, doc improvements) - [x] Did you read the [contributor guideline](https://github.com/pytorch/fairseq/blob/master/CONTRIBUTING.md)? - [x] Did you make sure to update the docs? - [x] Did you write any new necessary tests? ## What does this PR do? Fixes facebookresearch#1672 in part (part 1: [context](facebookresearch#1714 (comment))) ## PR review Anyone in the community is free to review the PR once the tests have passed. If we didn't discuss your PR in Github issues there's a high chance it will not be merged. ## Did you have fun? Make sure you had fun coding � Pull Request resolved: facebookresearch#1729 Differential Revision: D20049353 Pulled By: myleott fbshipit-source-id: 732077a1cc339c9f7ebe26dae42a7e8d7b5a07b4

Summary: # Before submitting - [x] Was this discussed/approved via a Github issue? (no need for typos, doc improvements) - [x] Did you read the [contributor guideline](https://github.com/pytorch/fairseq/blob/master/CONTRIBUTING.md)? - [x] Did you make sure to update the docs? - [x] Did you write any new necessary tests? ## What does this PR do? Fixes facebookresearch/fairseq#1672 in part (part 1: [context](facebookresearch/fairseq#1714 (comment))) ## PR review Anyone in the community is free to review the PR once the tests have passed. If we didn't discuss your PR in Github issues there's a high chance it will not be merged. ## Did you have fun? Make sure you had fun coding � Pull Request resolved: facebookresearch/fairseq#1729 Differential Revision: D20049353 Pulled By: myleott fbshipit-source-id: 732077a1cc339c9f7ebe26dae42a7e8d7b5a07b4

update base criterion to require from_args classmethod and remove arg…

118890c

…s as an attribute.

facebook-github-bot added the CLA Signed label Feb 16, 2020

erip added 28 commits February 16, 2020 15:43

update adaptive loss to incorporate new criterion interface.

035dcda

update binary cross entropy loss to incorporate new criterion interface.

afefb64

update cross entropy loss to incorporate new criterion interface.

a425590

update label-smoothed cross entropy loss to incorporate new criterion…

f3d5ea2

… interface.

update label-smoothed cross entropy w/ alignment loss to incorporate …

10f340e

…new criterion interface.

update legacy masked lm loss to incorporate new criterion interface.

4f762c7

update masked lm loss to incorporate new criterion interface.

e80cb49

update NAT loss to incorporate new criterion interface.

e5ca9dc

update sent prediction loss to incorporate new criterion interface.

655221d

update sent ranking loss to incorporate new criterion interface.

e09db73

update composite loss to incorporate new criterion interface.

c3e095c

use new from_args API in build_criterion.

06bc76d

fix bug in label smoothed cross entropy super call.

3f9640a

fix label smoothing tests.

1f34a84

fix cross entropy for ASR and accompanying test.

f68f59d

update base lr scheduler to require from_args classmethod and remove …

e92acdc

…args as an attribute.

update cosine lr scheduler to incorporate new lr scheduler interface.

a283986

update fixed schedule to incorporate new lr scheduler interface.

ee55cce

update noam schedule to incorporate new lr scheduler interface.

07665ac

update reduce_lr_on_plateau schedule to incorporate new lr scheduler …

2e949e5

…interface.

update polynomial decay schedule to incorporate new lr scheduler inte…

5d0d7dd

…rface.

update tristage schedule to incorporate new lr scheduler interface.

cfa8da4

update triangular schedule to incorporate new lr scheduler interface.

cbbca63

fix typo in lr on plateau from_args.

a570aec

add builder method to conform to registry magic lookup.

9abbf5e

add lrs attribute since the lr list was being pummeled by updating se…

6accaea

…lf.lr with a double.

fix issue where warmup_updates was not set.

02ae5ed

update base optimizer to require from_args classmethod and remove arg…

979e66a

…s as an attribute.

erip added 11 commits February 16, 2020 19:09

update adam to incorporate new optimizer interface.

597c1eb

update adamax to incorporate new optimizer interface.

56157f1

update bmuf to incorporate new optimizer interface.

1f84596

update fp16 optimizers to incorporate new optimizer interface.

be5381b

update fused LAMB to incorporate new optimizer interface.

549b4e2

update NAG to incorporate new optimizer interface.

3bbc905

update SGD to incorporate new optimizer interface.

3503c3a

fix initialization order for adafactor.

3859a95

fix initialization order for adadelta.

0609f06

update BPEs to require from_args classmethod and remove args as an at…

04757af

…tribute. TODO: make a base class?

update tokenizers to require from_args classmethod and remove args as…

8f9ce14

… an attribute. TODO: make a base class?

This was referenced Feb 21, 2020

refactor namespaces in criterion interface #1729

Closed

refactor namespaces in lr scheduler #1730

Closed

refactor namespaces in optimizer #1731

Closed

refactor namespaces in BPE #1732

Closed

refactor namespaces in tokenizers #1733

Closed

erip mentioned this pull request Feb 24, 2020

Refactor namespaces tasks #1743

Closed

4 tasks

erip closed this Oct 9, 2020

erip deleted the feature/refactor-namespaces branch October 9, 2020 18:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] refactor namespaces #1714

[WIP] refactor namespaces #1714

erip commented Feb 16, 2020 •

edited

Loading

myleott commented Feb 18, 2020

erip commented Feb 18, 2020

myleott commented Feb 22, 2020

erip commented Feb 24, 2020

erip commented Mar 2, 2020

[WIP] refactor namespaces #1714

[WIP] refactor namespaces #1714

Conversation

erip commented Feb 16, 2020 • edited Loading

Before submitting

What does this PR do?

PR review

Did you have fun?

myleott commented Feb 18, 2020

erip commented Feb 18, 2020

myleott commented Feb 22, 2020

erip commented Feb 24, 2020

erip commented Mar 2, 2020

erip commented Feb 16, 2020 •

edited

Loading