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

Drop Zygote dependency and reformat code #66

Merged
merged 1 commit into from
Apr 26, 2020

Conversation

devmotion
Copy link
Member

This PR removes the Zygote dependency, as discussed in #65 (comment). Moreover, currently sometimes the explicit ZygoteRules.@adjoint and ZygoteRules.pullback was used and sometimes only @adjoint and pullback. I reformatted the code (adhering to the 92 characters limit) so that now consistently the more explict ZygoteRules.@adjoint and ZygoteRules.pullback are used to show clearly that these implementations use Zygote.

@devmotion
Copy link
Member Author

BTW maybe we should stop testing Zygote on Julia 1.0 - tests run for almost 4 hours now whereas tests on Julia 1.4 were finished after a bit less than 2 hours. Zygote's README also states explicitly:

Zygote supports Julia 1.0 onwards, but we highly recommend using Julia 1.3 or later.

@mohamed82008
Copy link
Member

BTW maybe we should stop testing Zygote on Julia 1.0 - tests run for almost 4 hours now whereas tests on Julia 1.4 were finished after a bit less than 2 hours.

Happy to stop testing Zygote on Julia 1.0.

Copy link
Member

@mohamed82008 mohamed82008 left a comment

Choose a reason for hiding this comment

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

Nice work!

@mohamed82008 mohamed82008 merged commit 34abdf4 into TuringLang:master Apr 26, 2020
@devmotion devmotion deleted the zygote branch April 26, 2020 16:48
@devmotion
Copy link
Member Author

I removed the Zygote tests on Julia 1.0, unfortunately it seems Github actions don't respect [skip ci] in the commit message...

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