Skip to content

Fix uniqueness check in movedim#44307

Closed
peterbell10 wants to merge 1 commit intopytorch:masterfrom
peterbell10:movedim-unique
Closed

Fix uniqueness check in movedim#44307
peterbell10 wants to merge 1 commit intopytorch:masterfrom
peterbell10:movedim-unique

Conversation

@peterbell10
Copy link
Copy Markdown
Collaborator

@peterbell10 peterbell10 commented Sep 8, 2020

Noticed this bug in torch.movedim (#41480). std::unique only guarantees uniqueness for sorted inputs. The current check lets through non-unique values when they aren't adjacent to each other in the list, e.g. (0, 1, 0) wouldn't raise an exception and instead the algorithm fails later with an internal assert.

@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented Sep 8, 2020

💊 CI failures summary and remediations

As of commit cdad3a6 (more details on the Dr. CI page):


  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)

ci.pytorch.org: 1 failed


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 1 time.

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 8, 2020

Codecov Report

Merging #44307 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #44307   +/-   ##
=======================================
  Coverage   69.24%   69.24%           
=======================================
  Files         381      381           
  Lines       47573    47573           
=======================================
  Hits        32943    32943           
  Misses      14630    14630           

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 cce5982...cdad3a6. Read the comment docs.

Copy link
Copy Markdown
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

good catch, thank you!

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@zou3519 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@zou3519 merged this pull request in 129d52a.

bitfort pushed a commit that referenced this pull request Sep 11, 2020
Summary:
Noticed this bug in `torch.movedim` (#41480). [`std::unique`](https://en.cppreference.com/w/cpp/algorithm/unique) only guarantees uniqueness for _sorted_ inputs. The current check lets through non-unique values when they aren't adjacent to each other in the list, e.g. `(0, 1, 0)` wouldn't raise an exception and instead the algorithm fails later with an internal assert.

Pull Request resolved: #44307

Reviewed By: mrshenli

Differential Revision: D23598311

Pulled By: zou3519

fbshipit-source-id: fd6cc43877c42bb243cfa85341c564b6c758a1bf
xuzhao9 pushed a commit that referenced this pull request Sep 18, 2020
Summary:
Noticed this bug in `torch.movedim` (#41480). [`std::unique`](https://en.cppreference.com/w/cpp/algorithm/unique) only guarantees uniqueness for _sorted_ inputs. The current check lets through non-unique values when they aren't adjacent to each other in the list, e.g. `(0, 1, 0)` wouldn't raise an exception and instead the algorithm fails later with an internal assert.

Pull Request resolved: #44307

Reviewed By: mrshenli

Differential Revision: D23598311

Pulled By: zou3519

fbshipit-source-id: fd6cc43877c42bb243cfa85341c564b6c758a1bf
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Noticed this bug in `torch.movedim` (pytorch#41480). [`std::unique`](https://en.cppreference.com/w/cpp/algorithm/unique) only guarantees uniqueness for _sorted_ inputs. The current check lets through non-unique values when they aren't adjacent to each other in the list, e.g. `(0, 1, 0)` wouldn't raise an exception and instead the algorithm fails later with an internal assert.

Pull Request resolved: pytorch#44307

Reviewed By: mrshenli

Differential Revision: D23598311

Pulled By: zou3519

fbshipit-source-id: fd6cc43877c42bb243cfa85341c564b6c758a1bf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants