Skip to content

Conversation

ervteng
Copy link
Contributor

@ervteng ervteng commented Oct 28, 2020

Proposed change(s)

PyTorch changed the behavior of tensor conversion - previously, if dtype was None it used the default tensor type. Now, if None it will use the type of the thing that is being converted.

This PR forces everything to use float32. In addition, we cap the Torch version to 1.7.X so that we won't break when Torch 1.8.0 comes out and breaks something.

Types of change(s)

  • Bug fix
  • New feature
  • Code refactor
  • Breaking change
  • Documentation update
  • Other (please describe)

Checklist

  • Added tests that prove my fix is effective or that my feature works
  • Updated the changelog (if applicable)
  • Updated the documentation (if applicable)
  • Updated the migration guide (if applicable)

Other comments

@ervteng ervteng changed the title [bug-fix] Use float64 in GAIL tests [bug-fix] Use float64 when converting np.ndarray to torch.tensor, cap Torch version to 1.7.x Oct 28, 2020
"pyyaml>=3.1.0",
# Windows ver. of PyTorch doesn't work from PyPi
'torch>=1.6.0;platform_system!="Windows"',
'torch>=1.6.0,<1.8.0;platform_system!="Windows"',
Copy link
Contributor

Choose a reason for hiding this comment

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

Not directly related to this change, but can we put a link to the windows instructions (either to our docs or external docs) as a comment here?

Copy link
Contributor

@chriselion chriselion left a comment

Choose a reason for hiding this comment

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

Looks good. If the float32 restriction is a pain and doesn't provide any performance benefit, we can remove the checks for it.

@ervteng ervteng merged commit 75f8a2b into master Oct 29, 2020
@delete-merged-branch delete-merged-branch bot deleted the develop-fixfloat64test branch October 29, 2020 17:33
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2021
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