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

refactor: Use torch APIs for ravel, tile, and outer tensorlib methods #1354

Merged
merged 6 commits into from
Mar 7, 2021

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Mar 5, 2021

Description

Resolves #1349

Adopt the torch API for ravel, tile, and outer for easier comparisons across backends as the PyTorch v1.8.0 release notes comment

New functions (most of them to improve numpy compatibility)

Note that in the case of outer the v1.8.0 docs note that for torch.ger (the current function)

This function is deprecated and will be removed in a future PyTorch release. Use torch.outer() instead.

As a result also bump the minimum compatible release number of PyTorch to 1.8 so that these APIs exist.

Checklist Before Requesting Reviewer

  • Tests are passing (CI is failing for known reasons)
  • "WIP" removed from the title of the pull request
  • Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

  • Summarize commit messages into a comprehensive review of the PR
* Use torch.ravel, torch.tile, torch.outer for PyTorch backend tensorlib methods
   - https://pytorch.org/docs/1.8.0/generated/torch.ravel.html
   - https://pytorch.org/docs/1.8.0/generated/torch.tile.html
   - https://pytorch.org/docs/1.8.0/generated/torch.outer.html
* Set minimum compatible PyTorch release to v1.8 to ensure APIs

@matthewfeickert matthewfeickert added refactor A code change that neither fixes a bug nor adds a feature build Changes that affect the build system or external dependencies labels Mar 5, 2021
@matthewfeickert matthewfeickert self-assigned this Mar 5, 2021
@codecov
Copy link

codecov bot commented Mar 5, 2021

Codecov Report

Merging #1354 (0ea12a2) into master (04b6c86) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1354   +/-   ##
=======================================
  Coverage   97.50%   97.50%           
=======================================
  Files          63       63           
  Lines        3768     3768           
  Branches      538      538           
=======================================
  Hits         3674     3674           
  Misses         55       55           
  Partials       39       39           
Flag Coverage Δ
unittests 97.50% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/pyhf/tensor/pytorch_backend.py 98.27% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04b6c86...0ea12a2. Read the comment docs.

@matthewfeickert
Copy link
Member Author

@lukasheinrich @kratsg let me know if you want this to go into a later release and not touch this now as it bumps up the minimum required release torch to v1.8.0 — the latest. I think this would still be okay to have in a patch release, but let me know your thoughts.

@matthewfeickert matthewfeickert changed the title refactor: Use torch APIs for ravel and tile tensorlib methods refactor: Use torch APIs for ravel, tile, and outer tensorlib methods Mar 5, 2021
@matthewfeickert matthewfeickert added the docs Documentation related label Mar 5, 2021
Copy link
Contributor

@kratsg kratsg left a comment

Choose a reason for hiding this comment

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

just a "lol" and "lgtm" here

@matthewfeickert matthewfeickert added this to In progress in v0.6.1 via automation Mar 6, 2021
@matthewfeickert matthewfeickert merged commit 572fc69 into master Mar 7, 2021
v0.6.1 automation moved this from In progress to Done Mar 7, 2021
@matthewfeickert matthewfeickert deleted the feat/use-pytorch-ravel-tile-api branch March 7, 2021 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Changes that affect the build system or external dependencies docs Documentation related refactor A code change that neither fixes a bug nor adds a feature
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Use new APIs added in PyTorch v1.8.0
2 participants