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

no method matching MersenneTwister(::typeof(abs2), ::Array{Complex{Float64},1}) #227

Closed
baggepinnen opened this issue Jul 6, 2020 · 11 comments · Fixed by #228
Closed

no method matching MersenneTwister(::typeof(abs2), ::Array{Complex{Float64},1}) #227

baggepinnen opened this issue Jul 6, 2020 · 11 comments · Fixed by #228
Labels
bug Something isn't working

Comments

@baggepinnen
Copy link

baggepinnen commented Jul 6, 2020

I suddenly got a mysterious error on CI from a call to Zygote.gradient

LoadError: MethodError: no method matching MersenneTwister(::typeof(abs2), ::Array{Complex{Float64},1})

I haven't touched the code that threw the error in a long time so something must have gone wild in Zygote or ChainRules
link to CI failure: https://github.com/baggepinnen/SpectralDistances.jl/runs/840227039#step:4:721
link to failing call: https://github.com/baggepinnen/SpectralDistances.jl/blob/master/test/test_diff.jl#L204

Probably caused by 697e7e4

julia version 1.4.2
Installed ChainRules ─────────────────── v0.7.5
Installed Zygote ─────────────────────── v0.5.2
Installed ZygoteRules ────────────────── v0.2.0
baggepinnen referenced this issue Jul 6, 2020
* Nograd for MersenneTwister

* MersenneTwister frule

* Bumps patch version

* Bump patch version
@mcabbott
Copy link
Member

mcabbott commented Jul 6, 2020

Should this package test Zygote, to catch such things?

Possibly just @test (Pkg.test("Zygote"); true), but that takes a while so perhaps better a CI task done after the others, somehow.

@willtebbutt
Copy link
Member

Ideally we would, but Zygote has a fairly elaborate CI pipeline, so it would take some work to get this set up.

@mcabbott
Copy link
Member

mcabbott commented Jul 6, 2020

Why do you say elaborate? https://github.com/FluxML/Zygote.jl/blob/master/.travis.yml looks standard.

Or the @test above would work so long as Zygote was a test dep.

@simeonschaub
Copy link
Member

We could copy what we do in ChainRulesCore for ChainRules integration tests already. Would be good to have a separate runner for this, so the test suite doesn't take forever to run.

@willtebbutt
Copy link
Member

willtebbutt commented Jul 6, 2020

Why do you say elaborate? https://github.com/FluxML/Zygote.jl/blob/master/.travis.yml looks standard.

I guess I was more referring to the fact that they've got a separate gitlab CI (I believe for the benefit of GPU stuff?) run set up when you invoke bors: https://github.com/FluxML/Zygote.jl/blob/master/.gitlab-ci.yml

But I suppose a regular CI run that doesn't handle the GPU stuff would be an improvement over nothing.

@mcabbott
Copy link
Member

mcabbott commented Jul 6, 2020

OK, yes hooking up the GPU tests sounds harder. But the basic level would have caught this.

ChainRulesCore's Cirrus CI at https://github.com/JuliaDiff/ChainRulesCore.jl/blob/master/.cirrus.yml looks like exactly the right sort of thing.

@CarloLucibello
Copy link
Contributor

I think this is also hitting me in FluxML/Flux.jl#1264
see error here:
https://travis-ci.org/github/FluxML/Flux.jl/jobs/705291614

@CarloLucibello
Copy link
Contributor

+1 on running Zygote tests here as well

@nickrobinson251
Copy link
Contributor

Thanks for the report! And thanks @sethaxen for fixing it so fast. ChainRules v0.7.6 should be released any minute and should fix this 🤞

I opened another issue to discuss the idea of running Zygote tests as part of the ChainRules CI #229

@baggepinnen
Copy link
Author

My failing CI is now shining bright and green, thanks Seth and Co!

@DhairyaLGandhi
Copy link
Contributor

+1 for Zygote tests in ChainRules!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants