Skip to content

Conversation

@gpengzhi
Copy link
Collaborator

  1. Add PretrainedBase and modify the code structure of pre-trained modules.
  2. Resolve comments in Add XLNet Modules #107
  3. Resolve Multiple inheritance with DecoderBase #108
  4. Resolve Add configuration for XLNet-Base #106

@gpengzhi gpengzhi requested review from ZhitingHu and huzecong July 25, 2019 01:48
Copy link
Collaborator

@huzecong huzecong left a comment

Choose a reason for hiding this comment

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

Generally LGTM! Since the current solution is working I don't think we need to change the design, but here's an idea I've just had: it might be possible to make PretrainedBase a "mixin" class, i.e. a class that other classes inherit from for the sole purpose of adding a couple of methods and attributes, and whose constructor is not called. If you're familiar with strongly-typed languages, it's a bit like C++/Rust traits and Java interfaces with default implementations.

But anyway, let's keep it as is for now.

@gpengzhi gpengzhi requested a review from huzecong July 25, 2019 18:25
@huzecong huzecong merged commit 8679104 into asyml:master Jul 25, 2019
@ZhitingHu
Copy link
Member

The examples/xlnet example is yet to be simplified, right?

@gpengzhi
Copy link
Collaborator Author

@ZhitingHu Yes. I am working on it.

gpengzhi added a commit that referenced this pull request Jul 27, 2019
* Resolve comments
@gpengzhi gpengzhi deleted the xlnet branch July 27, 2019 21:59
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.

Multiple inheritance with DecoderBase Add configuration for XLNet-Base

3 participants