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

housekeeping related to upstreaming #41

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

jedbrown
Copy link
Collaborator

This is just tracking changes associated with the upstream merge and fixing of the temporary ReverseFirst kludge. I test using:

$ RUSTFLAGS='-Z autodiff=Enable' cargo +enzyme test

This passes all but one case:

test tests/ui/wrong_activity1.rs ... error
Expected test case to fail to compile, but it succeeded.

Note also that some previously-passing tests had to be disabled (see cfg(broken)).

@jedbrown jedbrown requested a review from ZuseZ4 March 19, 2025 04:19
@ZuseZ4
Copy link
Member

ZuseZ4 commented Mar 19, 2025

Thanks!
But it looks like by now enough tests are upstreamed to result in duplicated work by maintaining these tests here.
I therefore tend to upstream the remaining, larger test like your neohookean or ADBench.
Afterwards we could delete all testcases from this repo, and just keep the files for the website.
I don't know how picky people are about the increasing the runtime of our test suite, since Enzyme compile times are quite bad, I will let you know once I got feedback.

@jedbrown
Copy link
Collaborator Author

I'm glad those are upstream and would be happy to submit more involved test cases there. However, this documentation site will only be accurate if the code snippets are tested. That is, I think it would be a regression in reliability to move additional cases out and return to inlining code in the markdown source or having untested files here.

And I'd expect the pain of maintenance across the duplication to be much smaller than the hassle of making the rust-lang/rust repo a submodule or something like that to avoid duplication while still ensuring that our docs only reference actively-tested code.

@ZuseZ4
Copy link
Member

ZuseZ4 commented Mar 19, 2025

The issues you describe should only happen while we don't have ad in nightly though, or? I don't mind keeping them around for another month, till lib support landed. Once we have it in niggtly, we can just run the remaining doc tests on the website.

@jedbrown
Copy link
Collaborator Author

You mean running RUSTFLAGS='-Z autodiff=Enable' mdbook test? That was deemed too slow earlier (81470df), though rustdoc on edition 2024 has nominally solved that performance issue. If we observe no downside, we could go back to inlining. Note that I had to patch mdbook to deal with lto=fat. I could probably expose that with a configuration flag and submit it upstream if we choose mdbook test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants