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

Fix #627 #628

Merged
merged 1 commit into from
Apr 30, 2020
Merged

Fix #627 #628

merged 1 commit into from
Apr 30, 2020

Conversation

mohamed82008
Copy link
Contributor

This PR fixes Zygote loading on Julia 1.0.

@DhairyaLGandhi
Copy link
Member

We would need this for sure

bors r+

@mohamed82008
Copy link
Contributor Author

Let's please get a new release after this is merged.

@cossio
Copy link
Contributor

cossio commented Apr 30, 2020

Can we add a test for this? The definition before this PR:

Base.broadcasted(f, ps::Params) = broadcasted(f, ps.order)

was passing all tests, but it should be detected.

@bors
Copy link
Contributor

bors bot commented Apr 30, 2020

Build succeeded:

@bors bors bot merged commit 364fa91 into FluxML:master Apr 30, 2020
@mohamed82008
Copy link
Contributor Author

I am assuming tests don't run on Julia 1.0 here.

@DhairyaLGandhi
Copy link
Member

Is that something Julia 1.3+?

@cossio
Copy link
Contributor

cossio commented Apr 30, 2020

Travis is running tests on Julia 1.3, I think.

@mohamed82008
Copy link
Contributor Author

mohamed82008 commented Apr 30, 2020

I think it's just that broadcasted wasn't exported from Base.Broadcast in Julia 1.0. All you have to do to get the error in the issue is call using Zygote on Julia 1.0 :)

@cossio
Copy link
Contributor

cossio commented Apr 30, 2020

Hmmm, I have Zygote 0.4.18 and I do using Zygote without error.

@cossio
Copy link
Contributor

cossio commented Apr 30, 2020

Also, broadcasted is not exported still in Julia 1.4.

@mohamed82008
Copy link
Contributor Author

Hmmm, I have Zygote 0.4.18 and I do using Zygote without error.

On Julia 1.0?

Also, broadcasted is not exported still in Julia 1.4.

I think it is exported from Base.Broadcast to Base but not from Base.

julia> Base.broadcasted
broadcasted (generic function with 53 methods)

@cossio
Copy link
Contributor

cossio commented Apr 30, 2020

Oh you are right. I am running Julia 1.4. I misunderstood you.

@DhairyaLGandhi
Copy link
Member

I'd quite like registering on a green tick mark, if that's alright; just waiting on CI now.

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