Skip to content

CI is toasted #6138

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

Closed
NicolasHug opened this issue Jun 8, 2022 · 9 comments
Closed

CI is toasted #6138

NicolasHug opened this issue Jun 8, 2022 · 9 comments

Comments

@NicolasHug
Copy link
Member

NicolasHug commented Jun 8, 2022

From bf1914c on main branch:

$SRC_DIR/torchvision/csrc/ops/quantized/cpu/qnms_kernel.cpp:2:10: fatal error: ATen/native/quantized/affine_quantizer.h: No such file or directory
    2 | #include <ATen/native/quantized/affine_quantizer.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Because torch core renamed affine_quantizer.h into AffineQuantizer.h.

This will eventually be fixed by #6133. The irony is that some jobs are relying on the nightly from today while some other jobs are still relying on nightlies from yesterday where the changes from core haven't propagated. So these jobs are red on #6133.

I guess we'll have to wait till tomorrow to merge #6133 and get our green CI back.

cc @seemethere

@malfet
Copy link
Contributor

malfet commented Jun 8, 2022

We can add a script to create a symlink from one to another, if header is not available (Or #include one from another), but this is bad. Wonder if change on core were marked as BC breaking?

@seemethere
Copy link
Member

seemethere commented Jun 8, 2022

Doesn't appear so, pytorch/pytorch#77037

cc @dzdang

@malfet
Copy link
Contributor

malfet commented Jun 8, 2022

To copy-paste a comment from #6133 : jobs that are broken depend on torchdata, which nightly build failed yesterday, so it forces torch nightly installation prior to that change.

@dzdang
Copy link
Contributor

dzdang commented Jun 8, 2022

@malfet Sorry, just saw this. yeah, I think in retrospect, I should've marked the PR as bc-breaking. sorry about that (wasn't aware of existent projects that depended on the header file that wasn't part of the pytorch and fbcode codebase). do we need to revert the PR, or proceed with the solution above?

@malfet
Copy link
Contributor

malfet commented Jun 8, 2022

Also, looks like M1 CI is somehow affected by this as well:

2022-06-08T19:28:17.4266160Z /Users/ec2-user/runner/_work/vision/vision/torchvision/csrc/ops/quantized/cpu/qnms_kernel.cpp:2:10: fatal error: 'ATen/native/quantized/AffineQuantizer.h' file not found
2022-06-08T19:28:17.4267120Z #include <ATen/native/quantized/AffineQuantizer.h>
2022-06-08T19:28:17.4267400Z          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2022-06-08T19:28:17.4267580Z 1 error generated.

@malfet
Copy link
Contributor

malfet commented Jun 8, 2022

@malfet Sorry, just saw this. yeah, I think in retrospect, I should've marked the PR as bc-breaking. sorry about that (wasn't aware of existent projects that depended on the header file that wasn't part of the pytorch and fbcode codebase). do we need to revert the PR, or proceed with the solution above?

@dzdang I guess it's a question of whether we have any public facing tutorials/docs talking about AffineQuantizer, etc

@dzdang
Copy link
Contributor

dzdang commented Jun 8, 2022

@malfet We don't. I took a look at qnms_kernel.cpp, and I don't think I saw any dependencies in it that would require #include that header file. maybe we can just remove the #include?

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Jun 8, 2022

maybe we can just remove the #include?

Removed locally the include and can confirm that torchvision compiles and test_ops.py::test_qnms are passing.

malfet added a commit that referenced this issue Jun 9, 2022
NicolasHug pushed a commit that referenced this issue Jun 9, 2022
As @vfdev-5 proposed in #6138 (comment)

Co-authored-by: vfdev <vfdev.5@gmail.com>
@NicolasHug
Copy link
Member Author

#6141 merged, closing. Thanks everyone.

facebook-github-bot pushed a commit that referenced this issue Jun 9, 2022
Summary:
As vfdev-5 proposed in #6138 (comment)

Reviewed By: YosuaMichael

Differential Revision: D37038113

fbshipit-source-id: 3a63f0228ebac448e4ed4394c906fbd30b7f1030

Co-authored-by: vfdev <vfdev.5@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants