Skip to content

Add SaveNNMixin for fixing saving/loading of NNs #1012

Merged
merged 8 commits into from
Nov 29, 2022
Merged

Conversation

Mr-Geekman
Copy link
Contributor

@Mr-Geekman Mr-Geekman commented Nov 24, 2022

Before submitting (must do checklist)

  • Did you read the contribution guide?
  • Did you update the docs? We use Numpy format for all the methods and classes.
  • Did you write any new necessary tests?
  • Did you update the CHANGELOG?

Proposed Changes

Look at #1011.

Closing issues

Closes #1011.

@Mr-Geekman Mr-Geekman self-assigned this Nov 24, 2022
@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2022

Codecov Report

❗ No coverage uploaded for pull request base (inference-v2@75c4623). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             inference-v2    #1012   +/-   ##
===============================================
  Coverage                ?   85.91%           
===============================================
  Files                   ?      163           
  Lines                   ?     8700           
  Branches                ?        0           
===============================================
  Hits                    ?     7475           
  Misses                  ?     1225           
  Partials                ?        0           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Nov 24, 2022

@github-actions github-actions bot temporarily deployed to pull request November 24, 2022 15:55 Inactive
@github-actions github-actions bot temporarily deployed to pull request November 24, 2022 16:34 Inactive
@github-actions github-actions bot temporarily deployed to pull request November 25, 2022 08:22 Inactive
@github-actions github-actions bot temporarily deployed to pull request November 25, 2022 10:15 Inactive
@github-actions github-actions bot temporarily deployed to pull request November 25, 2022 10:54 Inactive
@github-actions github-actions bot temporarily deployed to pull request November 25, 2022 11:19 Inactive
@martins0n martins0n self-requested a review November 28, 2022 07:35
def test_save_load(example_tsds):
horizon = 3
model = TFTModel(max_epochs=2, learning_rate=[0.1], gpus=0, batch_size=64, loss=MAEPF())
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we change loss?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I probably made a mistake. As you can see, this loss is only imported in the previous test and wasn't imported in the whole test file. There are two ways to fix it:

  • Import MAEPF in this test too.
  • Use default loss.

I choose the second option.

@Mr-Geekman Mr-Geekman merged commit 3a40f75 into inference-v2 Nov 29, 2022
@Mr-Geekman Mr-Geekman deleted the issue-1011 branch November 29, 2022 15:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants