-
Notifications
You must be signed in to change notification settings - Fork 13.3k
autodiff fails in impl blocks #139557
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
Comments
@rustbot claim |
@SpencerWightman Did you have a chance to look into the issue, or do you have any questions? |
@ZuseZ4 Yes, I've made some progress. Sorry for holding it up so long. I'll make a PR today or drop the claim. |
Hi @SpencerWightman, I'm currently exploring autodiff and would be happy to collaborate on this if you're interested. Otherwise, please let me know if it's okay for me to take it on—I'm excited to contribute either way! |
@SpencerWightman no rush if you don't get to it today, but I hope to have a PR for this ready around the 22nd, so it has a bit of time to land before I enable autodiff on the 26th. Feel free to experiment with it a bit more, otherwise I can probably come up with a fix on the 22nd. @Shourya742 I pinged you in another issue with something you could look at if you have time. |
@rustbot release-assignment |
[DO NOT MERGE] start building enzyme on x86_64-gnu-llvm-{19|20} builders My goal is to put this in CI on April 26, to have a week to land some of the outstanding PRs: rust-lang#139700 rust-lang#139308 rust-lang#139557 rust-lang#140030 rust-lang#140049 The autodiff flags PR should land first, but otherwise they don't overlap and are mostly ready, so it shouldn't be too hard to land them. In the meantime, I'll experiment here with some builders. r? `@oli-obk` Tracking: - rust-lang#124509 try-job: x86_64-gnu-llvm-19 try-job: x86_64-gnu-llvm-20
[DO NOT MERGE] start building enzyme on x86_64-gnu-llvm-{19|20} builders My goal is to put this in CI on April 26, to have a week to land some of the outstanding PRs: rust-lang#139700 rust-lang#139308 rust-lang#139557 rust-lang#140030 rust-lang#140049 The autodiff flags PR should land first, but otherwise they don't overlap and are mostly ready, so it shouldn't be too hard to land them. In the meantime, I'll experiment here with some builders. r? `@oli-obk` Tracking: - rust-lang#124509 try-job: dist-x86_64-linux
@ZuseZ4, Is it ok now, if I take this up now? |
@rustbot claim |
…ils-on-impl-block, r=ZuseZ4 Fix auto diff failing on inherent impl blocks closes: rust-lang#139557 r? `@ZuseZ4`
Rollup merge of rust-lang#140104 - Shourya742:2025-04-21-auto-diff-fails-on-impl-block, r=ZuseZ4 Fix auto diff failing on inherent impl blocks closes: rust-lang#139557 r? ``@ZuseZ4``
I tried this code:
https://github.com/EnzymeAD/rustbook/blob/main/samples/tests/traits/mod.rs
I expected to see this happen: Passes the assertion
Instead, this happened:
Meta
rustc --version --verbose
:Backtrace
This is an easy bug and a got way to get started. It used to work a few weeks ago, so it's likely that just one of the other improvement PRs accidentally regressed this test, since it's not upstream yet. Supporting autodiff in impl blocks is of course important, so it would be nice to fix this regression.
To prevent further regressions, this autodiff invocation (under impl Volumentric for Ogden) should be added in
tests/pretty/autodiff
. We emit the error asdcx.emit_err(errors::AutoDiffInvalidApplication { span: item.span() });
incompiler/rustc_builtin_macros/src/autodiff.rs
so the bugfix likely goes there, this PR could be an inspiration: https://github.com/rust-lang/rust/pull/138314/filesTracking:
The text was updated successfully, but these errors were encountered: