Skip to content

Conversation

@VinodSKumar
Copy link
Contributor

@VinodSKumar VinodSKumar commented Jul 21, 2020

Replaced "whitelist" from default_mappings.py
Fixes #41756

@dr-ci
Copy link

dr-ci bot commented Jul 21, 2020

💊 CI failures summary and remediations

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


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

ci.pytorch.org: 2 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 25 times.

@zhangguanheng66 zhangguanheng66 requested a review from z-a-f July 21, 2020 18:16
@zhangguanheng66 zhangguanheng66 added oncall: quantization Quantization support in PyTorch triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Jul 21, 2020
@z-a-f
Copy link

z-a-f commented Aug 5, 2020

Please, rebase :)

Copy link

@z-a-f z-a-f left a comment

Choose a reason for hiding this comment

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

Will recheck after rebase

@VinodSKumar VinodSKumar force-pushed the remove_black_white_list branch from 365a2d3 to c6beaa3 Compare August 10, 2020 16:09
@VinodSKumar
Copy link
Contributor Author

@z-a-f rebase has been done

@VinodSKumar VinodSKumar requested a review from z-a-f August 11, 2020 09:41
@VinodSKumar VinodSKumar force-pushed the remove_black_white_list branch from ce24eb0 to baff294 Compare August 11, 2020 12:56
@VinodSKumar
Copy link
Contributor Author

@z-a-f rebased again to since one CI was failing due to base env.

@VinodSKumar VinodSKumar force-pushed the remove_black_white_list branch from baff294 to b0467ae Compare August 12, 2020 15:30
@VinodSKumar
Copy link
Contributor Author

Resolved new conflicts and rebased

@VinodSKumar VinodSKumar force-pushed the remove_black_white_list branch from b0467ae to d072182 Compare August 13, 2020 03:36
@VinodSKumar
Copy link
Contributor Author

Rebased again to resolve conflicts

Copy link
Contributor

@malfet malfet left a comment

Choose a reason for hiding this comment

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

You've forgot to rename the argument name
Also, you still have the merge conflicts (please feel free to post git commands you are using to rebase your change here)

@VinodSKumar
Copy link
Contributor Author

You've forgot to rename the argument name
Also, you still have the merge conflicts (please feel free to post git commands you are using to rebase your change here)

@malfet This PR was for the issue 41756 relating to default_mappings.py, I presumed that it should only be related to it. Thanks for the feedback will update the argument and its usage.

Regarding the rebase steps, please find the history of git commands used in this branch, Please let me know if any more information is required.
git checkout remove_black_white_list
git fetch upstream
git rebase upstream/master
git add torch/quantization/quantize.py
git add torch/testing/_internal/common_quantization.py
git commit -m "Resolve conflicts from master"
git rebase --abort //due to the commit, the changes were not there in the staging area, hence restarted the rebasing.
git fetch upstream
git rebase upstream/master
git add torch/quantization/quantize.py
git add torch/testing/_internal/common_quantization.py
git rebase --continue
git push origin remove_black_white_list -f

// second time rebasing for new conflicts, usually below are the steps for rebasing
git fetch upstream
git rebase upstream/master
git add torch/testing/_internal/common_quantization.py
git rebase --continue
git push origin remove_black_white_list -f

@VinodSKumar VinodSKumar force-pushed the remove_black_white_list branch 2 times, most recently from aa78c8f to 756c1ac Compare August 17, 2020 18:30
@VinodSKumar VinodSKumar requested a review from malfet August 18, 2020 17:38
@z-a-f
Copy link

z-a-f commented Aug 21, 2020

LGTM, will leave to @malfet for landing

Copy link

@z-a-f z-a-f left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
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.

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

@VinodSKumar VinodSKumar force-pushed the remove_black_white_list branch from 756c1ac to 67b4e00 Compare September 4, 2020 01:42
@VinodSKumar
Copy link
Contributor Author

Rebased to double check the PR changes

@codecov
Copy link

codecov bot commented Sep 4, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@538d3bd). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #41802   +/-   ##
=========================================
  Coverage          ?   69.35%           
=========================================
  Files             ?      381           
  Lines             ?    47321           
  Branches          ?        0           
=========================================
  Hits              ?    32821           
  Misses            ?    14500           
  Partials          ?        0           

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 538d3bd...67b4e00. Read the comment docs.

Copy link
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.

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

@facebook-github-bot
Copy link
Contributor

@malfet merged this pull request in 2a1fc56.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged oncall: quantization Quantization support in PyTorch open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace blacklist/whitelist in torch/quantization/default_mappings.py

7 participants