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

[MRG] Fix instantiation of ValFunction (which raises a warning with PyTorch) #338

Merged
merged 3 commits into from
Jan 19, 2022

Conversation

Pseudomanifold
Copy link
Contributor

ValFunction should not be instantiated since autograd functions are
supposed to only ever use static methods. This solves a warning message
raised by PyTorch.

The warning message of PyTorch indicates that this behaviour may raise
an error in future versions.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Motivation and context / Related issue

With PyTorch 1.10.1+cu102, using OT on a tensor with a gradient, the following warning message is raised:

[....] valfunction should not be instantiated. Methods on autograd functions are all static, so you should
invoke them on the class itself. Instantiating an autograd function will raise an error in a future version
of PyTorch.

This pull request fixes the instantiation, thus also fixing issue #337.

How has this been tested (if it applies)

Used my own code with the fork of POT. The warning is not raised any more; further behaviour remains exactly the same.

PR checklist

  • I have read the CONTRIBUTING document.
  • The documentation is up-to-date with the changes I made (check build artifacts).
  • All tests passed, and additional code has been covered with new tests.
  • I have added the PR and Issue fix to the RELEASES.md file.

`ValFunction` should not be instantiated since `autograd` functions are
supposed to only ever use static methods. This solves a warning message
raised by PyTorch.
@codecov
Copy link

codecov bot commented Jan 19, 2022

Codecov Report

Merging #338 (59b6564) into master (5861209) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #338      +/-   ##
==========================================
+ Coverage   93.03%   93.07%   +0.03%     
==========================================
  Files          21       21              
  Lines        5198     5198              
==========================================
+ Hits         4836     4838       +2     
+ Misses        362      360       -2     

@rflamary rflamary merged commit 263c584 into PythonOT:master Jan 19, 2022
@rflamary
Copy link
Collaborator

Thank you very much @Pseudomanifold

@Pseudomanifold
Copy link
Contributor Author

You are very welcome :-) Thanks for the quick response and for providing this package. It has been of tremendous help so far!

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.

2 participants