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

Expand unthunk-tangent with more methods #28

Merged
merged 3 commits into from
Jan 20, 2025

Conversation

pxl-th
Copy link
Member

@pxl-th pxl-th commented Jan 19, 2025

These are moved over from Zygote:
https://github.com/FluxML/Zygote.jl/blob/a38a4a565b925723386d311938226e6e29a106c2/src/compiler/chainrules.jl#L3

PR Checklist

  • Tests are added
  • Documentation, if applicable

Copy link
Member

@ToucheSir ToucheSir left a comment

Choose a reason for hiding this comment

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

Thanks!

@ToucheSir ToucheSir merged commit a4d77ab into FluxML:master Jan 20, 2025
12 of 15 checks passed
@pxl-th pxl-th deleted the pxl-th/unthunk-tangent branch January 20, 2025 09:32
@pxl-th
Copy link
Member Author

pxl-th commented Jan 20, 2025

Ah... there's slight issue. wrap_chainrules_output is not defined in ZygoteRules. Not sure how I missed it :(
If we are going to deprecate this package, maybe we should instead move all method definitions to Zygote and leave only function definition in ZygoteRules?

@pxl-th
Copy link
Member Author

pxl-th commented Jan 20, 2025

If this is acceptable, I have opened a PR:
#29

@pxl-th
Copy link
Member Author

pxl-th commented Jan 20, 2025

Alternatively, we can move wrap_chainrules_output function definition to ZygoteRules...

@pxl-th
Copy link
Member Author

pxl-th commented Jan 20, 2025

Or just move on with FluxML/Zygote.jl#1551 without making changes for now (and try getting rid of ZygoteRules in future PRs).

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