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

Tapir -> Mooncake #338

Merged
merged 5 commits into from
Oct 29, 2024
Merged

Conversation

penelopeysm
Copy link
Member

@penelopeysm penelopeysm commented Oct 29, 2024

See #337 (comment) for context.

@penelopeysm penelopeysm changed the base branch from py/inline-import-rules to master October 29, 2024 03:13
@penelopeysm penelopeysm changed the base branch from master to py/inline-import-rules October 29, 2024 03:13
Copy link
Member

@mhauru mhauru left a comment

Choose a reason for hiding this comment

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

LGTM

@willtebbutt
Copy link
Member

Re the Mooncake failure -- I knew about this possibility previously, but this is the first time I've seen it in the wild. Sadly, I'm not 100% sure where to start on fixing this. See compintell/Mooncake.jl#319 for more context.

@penelopeysm
Copy link
Member Author

I'll mark the Mooncake test as broken and then we can go from there. Thanks both!

Comment on lines +81 to 88
if (AD == "All" || AD == "Mooncake") && VERSION >= v"1.10"
try
Mooncake.build_rrule(f, x)
catch exc
# TODO(penelopeysm):
# @test_throws AssertionError (expr...) doesn't work, unclear why
@test exc isa AssertionError
end
Copy link
Member Author

@penelopeysm penelopeysm Oct 29, 2024

Choose a reason for hiding this comment

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

I don't know why @test_throws doesn't work here, it's completely inexplicable. (When I use @test_throws, it doesn't throw an exception, so @test_throws fails)

However, I also didn't want to spend another 2 hours debugging this, so I'm just reverting to catching the expression and testing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW, we could simply exclude Mooncake from testing entirely, but I prefer having this as it will let us know when the bug above is fixed.

@penelopeysm
Copy link
Member Author

Remaining failing tests are due to Enzyme, I'm going to go ahead and merge first.

@penelopeysm penelopeysm merged commit 2ee8a5d into py/inline-import-rules Oct 29, 2024
22 of 26 checks passed
@penelopeysm penelopeysm deleted the py/inline-import-and-mooncake branch October 29, 2024 16:52
penelopeysm added a commit that referenced this pull request Oct 29, 2024
* Disable fail-fast on CI

* Inline expanded frule and rrule in BijectorsEnzymeExt

* Bump patch version

* Remove BijectorsEnzymeExt on 1.11.1+

* Tapir -> Mooncake (#338)

* Tapir -> Mooncake

* Bump minor version

* Mark Mooncake test as broken

* Remove BijectorsEnzymeExt on 1.11.1+

* Increase tolerance on `ordered` 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.

3 participants