Pass Opt to TransformerEncoder/Decoder #3486
Conversation
|
....and what will happen with parlai_internal? |
I'm getting an internal PR ready now! |
| :param bool embeddings_scale: Scale embeddings relative to their dimensionality. | ||
| Found useful in fairseq. | ||
| :param bool reduction: If true, returns the mean vector for the entire encoding | ||
| sequence. | ||
| :param int n_positions: |
There was a problem hiding this comment.
will these comments appear somewhere else then?
There was a problem hiding this comment.
In the opt declaration! I just noticed that not all of opt flags have descriptions, though, so I'll fill them in from here where applicable.
There was a problem hiding this comment.
i suppose that's one thing we lose by getting rid of this, but the net negative lines is so enticing...
👍 |
|
Hm, Transresnet tests are failing consistently. That's concerning... Will need to get to the bottom of that before merging. |
There was a problem hiding this comment.
I think this looks good - definitely a fan of net negative lines.
I think Jason made a good point about the field/attr descriptions. I like how they're in the opt, but would it also be possible to include a note in the docstring of where to find those descriptions? like, "For more information regarding model parameters, see :file:" or something like that
| :param bool embeddings_scale: Scale embeddings relative to their dimensionality. | ||
| Found useful in fairseq. | ||
| :param bool reduction: If true, returns the mean vector for the entire encoding | ||
| sequence. | ||
| :param int n_positions: |
There was a problem hiding this comment.
i suppose that's one thing we lose by getting rid of this, but the net negative lines is so enticing...
|
Big improvement. |
Patch description
Since the RFC was well received (#3470) here's a PR against master (the other PR was branched off of tf-components #3466).
Testing steps
Make sure CircleCI passes.