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

Correct layers order in tabular learner #3314

Merged
merged 1 commit into from
Apr 30, 2021
Merged

Conversation

gradientsky
Copy link
Contributor

@gradientsky gradientsky commented Apr 10, 2021

It looks like the layers ordering in tabular models changed between fastai v1 and v2 and this is negatively impacting performance (we tested on AutoML Benchmark datasets):

 framework  time_train_s  metric_error  time_infer_s  bestdiff  loss_rescaled  time_infer_s_rescaled      rank  rank=1_count
v1 layers   3422.424138  70027.763706     78.873793  0.016476       0.394089               1.145908  1.433005           531
v2 layers   3420.214877  71086.459576     81.902562  0.023838       0.533990               1.318654  1.566995           395

In v2 the default layers order is causing shift of layers sequence: 2x batchnorm layers at the top (harmless, but has no value) and Linear-RELU-Linear without batchnorm at the output.

v1 was using the following layers order:

TabularModel(
  (embeds): ModuleList(
    (0): Embedding(11, 6)
    (1): Embedding(18, 8)
    (2): Embedding(9, 5)
    (3): Embedding(17, 8)
    (4): Embedding(8, 5)
    (5): Embedding(7, 5)
    (6): Embedding(4, 3)
    (7): Embedding(44, 13)
  )
  (emb_drop): Dropout(p=0.1, inplace=False)
  (bn_cont): BatchNorm1d(6, eps=1e-05, momentum=0.1, affine=True, track_running_stats=True)
  (layers): Sequential(
    (0): Linear(in_features=59, out_features=200, bias=True)
    (1): ReLU(inplace=True)
    (2): BatchNorm1d(200, eps=1e-05, momentum=0.1, affine=True, track_running_stats=True)
    (3): Dropout(p=0.1, inplace=False)
    (4): Linear(in_features=200, out_features=100, bias=True)
    (5): ReLU(inplace=True)
    (6): BatchNorm1d(100, eps=1e-05, momentum=0.1, affine=True, track_running_stats=True)
    (7): Dropout(p=0.1, inplace=False)
    (8): Linear(in_features=100, out_features=2, bias=True)
  )
)

v2 layers order:

TabularModel(
  (embeds): ModuleList(
    (0): Embedding(11, 6)
    (1): Embedding(18, 8)
    (2): Embedding(9, 5)
    (3): Embedding(17, 8)
    (4): Embedding(8, 5)
    (5): Embedding(7, 5)
    (6): Embedding(4, 3)
    (7): Embedding(44, 13)
  )
  (emb_drop): Dropout(p=0.1, inplace=False)
  (bn_cont): BatchNorm1d(6, eps=1e-05, momentum=0.1, affine=True, track_running_stats=True)
  (layers): Sequential(
    (0): LinBnDrop(
      (0): BatchNorm1d(59, eps=1e-05, momentum=0.1, affine=True, track_running_stats=True)
      (1): Dropout(p=0.1, inplace=False)
      (2): Linear(in_features=59, out_features=200, bias=False)
      (3): ReLU(inplace=True)
    )
    (1): LinBnDrop(
      (0): BatchNorm1d(200, eps=1e-05, momentum=0.1, affine=True, track_running_stats=True)
      (1): Dropout(p=0.1, inplace=False)
      (2): Linear(in_features=200, out_features=100, bias=False)
      (3): ReLU(inplace=True)
    )
    (2): LinBnDrop(
      (0): Linear(in_features=100, out_features=2, bias=True)
    )
  )
)

@gradientsky gradientsky requested a review from jph00 as a code owner April 10, 2021 01:37
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@muellerzr
Copy link
Contributor

Looks good to me, I agree with the fix and change. cc @jph00

@jph00
Copy link
Member

jph00 commented Apr 30, 2021

Very well spotted, thanks!

@jph00 jph00 merged commit dea82c7 into fastai:master Apr 30, 2021
@hamelsmu hamelsmu added the bug label Apr 30, 2021
@hamelsmu hamelsmu changed the title Fixing layers order in tabular learner Correct layers order in tabular learner Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants