Fix for Zygote 0.6.30 breaking our tests #409

st-- · 2021-11-19T13:07:58Z

Some change in Zygote 0.6.30 seems to break our tests.

willtebbutt

Assuming that tests pass, please bump the patch and merge this.

edit: I guess we don't need a patch bump as it's just part of the tests. Please just merge when tests pass.

theogf · 2021-11-19T13:12:44Z

I would put my money on FluxML/Zygote.jl#785

codecov · 2021-11-19T13:13:13Z

Codecov Report

Merging #409 (b135c7a) into master (33d64d1) will decrease coverage by 19.68%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master     #409       +/-   ##
===========================================
- Coverage   89.23%   69.54%   -19.69%     
===========================================
  Files          52       52               
  Lines        1198     1192        -6     
===========================================
- Hits         1069      829      -240     
- Misses        129      363      +234

Impacted Files	Coverage Δ
src/basekernels/gabor.jl	`0.00% <0.00%> (-100.00%)`	⬇️
src/basekernels/cosine.jl	`0.00% <0.00%> (-100.00%)`	⬇️
src/basekernels/nn.jl	`0.00% <0.00%> (-97.62%)`	⬇️
src/basekernels/sm.jl	`0.00% <0.00%> (-94.12%)`	⬇️
src/basekernels/wiener.jl	`0.00% <0.00%> (-92.86%)`	⬇️
src/basekernels/piecewisepolynomial.jl	`0.00% <0.00%> (-89.48%)`	⬇️
src/basekernels/rational.jl	`13.33% <0.00%> (-80.00%)`	⬇️
src/basekernels/exponentiated.jl	`0.00% <0.00%> (-80.00%)`	⬇️
src/basekernels/matern.jl	`20.83% <0.00%> (-75.17%)`	⬇️
src/basekernels/constant.jl	`25.00% <0.00%> (-75.00%)`	⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33d64d1...b135c7a. Read the comment docs.

devmotion · 2021-11-19T13:20:43Z

It's nice to fix the tests but unfortunately it won't help users with newer Zygote versions. Did anyone check if the type inference problem causes a performance regression?

devmotion · 2021-11-19T13:25:10Z

Generally, I think it would better to mark the failing test as broken since then 1) we don't test the package with an outdated Zygote version (the latest version might fix or cause some other issues without us noticing it) and 2) we notice if the problem is fixed upstream without following the Zygote development.

test/utils.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

st-- · 2021-11-22T15:19:36Z

we notice if the problem is fixed upstream without following the Zygote development.

@devmotion unfortunately, it seems like @test_broken @inferred does not actually fail when @inferred passes again, so we'll still have to track this manually :(

devmotion · 2021-11-22T16:16:03Z

Can we use something like @test_throws ErrorException @inferred(...)?

theogf · 2021-11-24T15:07:47Z

Can we get this fast-forwarded? It blocks the rest of the PRs

test/test_utils.jl

st-- · 2021-11-25T08:26:21Z

@devmotion thanks, I didn't think of that 🤦 and only just saw the message. Good it's sorted now!

st-- · 2021-11-25T08:28:32Z

Would've just added a comment to the @test_throws to say "this is because test is broken, not because we actually want to ensure that this throws an error" - though hopefully we'll remember it anyways as soon as Zygote fixes it ;)

Zygote AD failures: * revert #409 (test_utils workaround for broken Zygote - now working again) * disable broken Zygote AD test for ChainTransform Improved tests: * finer-grained testsets * add missing test cases to test_AD * replace test_FiniteDiff with test_AD(..., :FiniteDiff, ...) * remove code duplication

commit f9bbd84 Author: st-- <st--@users.noreply.github.com> Date: Fri Jan 28 09:11:50 2022 +0100 make nystrom work with AbstractVector (#427) * make nystrom work with AbstractVector * add test * Update test/approximations/nystrom.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * patch bump * Update test/approximations/nystrom.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Apply suggestions from code review * deprecate * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com> * Update src/approximations/nystrom.jl Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com> * Update src/approximations/nystrom.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: David Widmann <devmotion@users.noreply.github.com> Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com> commit d1c68a9 Author: st-- <st--@users.noreply.github.com> Date: Thu Jan 13 22:33:43 2022 +0100 fix Distances compat (#423) * CompatHelper: bump compat for Distances to 0.10 for package test, (keep existing compat) * try out Theo's fix * fix test compat * use ForwardDiff for chain rule test of SqMahalanobis * test on 1.4 instead of 1.3 - see if the chainrules test passes there * revert version branch * revert to 1.3 * test_broken for older Julia versions Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> commit 93d33c2 Author: st-- <st--@users.noreply.github.com> Date: Wed Jan 12 14:11:14 2022 +0100 fix figure & cleanup (#422) * fix figure & cleanup * bump LIBSVM compat & Manifest * improve writing, replaces #321 commit 40cb59e Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed Jan 12 09:39:01 2022 +0200 CompatHelper: bump compat for Kronecker to 0.5 for package docs, (keep existing compat) (#367) * CompatHelper: bump compat for Kronecker to 0.5 for package docs, (keep existing compat) * ] up Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> Co-authored-by: st-- <st--@users.noreply.github.com> commit 7204529 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue Jan 11 18:37:23 2022 +0200 CompatHelper: bump compat for Kronecker to 0.5 for package test, (keep existing compat) (#366) Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> Co-authored-by: st-- <st--@users.noreply.github.com> commit 924925d Author: st-- <st--@users.noreply.github.com> Date: Tue Jan 11 16:26:02 2022 +0100 switch SVM example to half-moon dataset (#421) commit 992b665 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri Dec 24 12:18:56 2021 +0200 CompatHelper: bump compat for SpecialFunctions to 2 for package test, (keep existing compat) (#412) Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com> Co-authored-by: st-- <st--@users.noreply.github.com> commit 04fa7f7 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu Dec 23 13:33:59 2021 +0200 CompatHelper: bump compat for SpecialFunctions to 2, (keep existing compat) (#411) Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com> Co-authored-by: st-- <st--@users.noreply.github.com> commit c0fc3e1 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu Dec 23 01:10:40 2021 +0200 CompatHelper: add new compat entry for Compat at version 3 for package test, (keep existing compat) (#418) Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> commit 05fe340 Author: st-- <st--@users.noreply.github.com> Date: Tue Dec 21 00:49:37 2021 +0200 use only() instead of first() (#403) * use only() instead of first() for 1-"vectors" that were for the benefit of Flux * fix one test that should not have worked as it was * add missing scalar Sinus constructor commit 2d17212 Author: st-- <st--@users.noreply.github.com> Date: Sat Dec 18 23:43:30 2021 +0200 Zygote AD failure workarounds & test cleanup (#414) Zygote AD failures: * revert #409 (test_utils workaround for broken Zygote - now working again) * disable broken Zygote AD test for ChainTransform Improved tests: * finer-grained testsets * add missing test cases to test_AD * replace test_FiniteDiff with test_AD(..., :FiniteDiff, ...) * remove code duplication commit 3c49949 Author: Théo Galy-Fajou <theo.galyfajou@gmail.com> Date: Wed Nov 24 18:32:19 2021 +0100 Fix typo in valid_inputs error (#408) * Fix typo in valid_inputs error * Update src/utils.jl Co-authored-by: David Widmann <devmotion@users.noreply.github.com> Co-authored-by: David Widmann <devmotion@users.noreply.github.com> commit 9955044 Author: st-- <st--@users.noreply.github.com> Date: Wed Nov 24 18:55:18 2021 +0200 Fix for Zygote 0.6.30 breaking our tests (#409) * restrict Zygote to <0.6.30 * revert Zygote test restriction and add finer-grained testset * Update test/utils.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * revert testset * mark test_broken * Use `@test_throws` instead of `@test_broken` Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: David Widmann <devmotion@users.noreply.github.com> commit 33d64d1 Author: Théo Galy-Fajou <theo.galyfajou@gmail.com> Date: Thu Nov 4 14:23:57 2021 +0100 Add benchmarking CI (#399) * Add benchmark file * delete old benchmarks * Add github action * Add Project * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Missing end of line Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> commit 360ce10 Author: David Widmann <devmotion@users.noreply.github.com> Date: Tue Nov 2 11:09:58 2021 +0100 Update docstring of `GibbsKernel` (#395)

* initial version of training kernel parameters example from st/examples (#234) * update script * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix out-of-domain initial value * initial version of training kernel parameters example from st/examples (#234) * update script * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix out-of-domain initial value * Add ParameterHandling * Extend example * Failed attempts with default Flux opt * Add Flux.destructure example * Add some nothing * Squashed commit of the following: commit f9bbd84 Author: st-- <st--@users.noreply.github.com> Date: Fri Jan 28 09:11:50 2022 +0100 make nystrom work with AbstractVector (#427) * make nystrom work with AbstractVector * add test * Update test/approximations/nystrom.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * patch bump * Update test/approximations/nystrom.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Apply suggestions from code review * deprecate * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: David Widmann <devmotion@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com> * Update src/approximations/nystrom.jl Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com> * Update src/approximations/nystrom.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: David Widmann <devmotion@users.noreply.github.com> Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com> commit d1c68a9 Author: st-- <st--@users.noreply.github.com> Date: Thu Jan 13 22:33:43 2022 +0100 fix Distances compat (#423) * CompatHelper: bump compat for Distances to 0.10 for package test, (keep existing compat) * try out Theo's fix * fix test compat * use ForwardDiff for chain rule test of SqMahalanobis * test on 1.4 instead of 1.3 - see if the chainrules test passes there * revert version branch * revert to 1.3 * test_broken for older Julia versions Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> commit 93d33c2 Author: st-- <st--@users.noreply.github.com> Date: Wed Jan 12 14:11:14 2022 +0100 fix figure & cleanup (#422) * fix figure & cleanup * bump LIBSVM compat & Manifest * improve writing, replaces #321 commit 40cb59e Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed Jan 12 09:39:01 2022 +0200 CompatHelper: bump compat for Kronecker to 0.5 for package docs, (keep existing compat) (#367) * CompatHelper: bump compat for Kronecker to 0.5 for package docs, (keep existing compat) * ] up Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> Co-authored-by: st-- <st--@users.noreply.github.com> commit 7204529 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue Jan 11 18:37:23 2022 +0200 CompatHelper: bump compat for Kronecker to 0.5 for package test, (keep existing compat) (#366) Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> Co-authored-by: st-- <st--@users.noreply.github.com> commit 924925d Author: st-- <st--@users.noreply.github.com> Date: Tue Jan 11 16:26:02 2022 +0100 switch SVM example to half-moon dataset (#421) commit 992b665 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri Dec 24 12:18:56 2021 +0200 CompatHelper: bump compat for SpecialFunctions to 2 for package test, (keep existing compat) (#412) Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com> Co-authored-by: st-- <st--@users.noreply.github.com> commit 04fa7f7 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu Dec 23 13:33:59 2021 +0200 CompatHelper: bump compat for SpecialFunctions to 2, (keep existing compat) (#411) Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com> Co-authored-by: st-- <st--@users.noreply.github.com> commit c0fc3e1 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu Dec 23 01:10:40 2021 +0200 CompatHelper: add new compat entry for Compat at version 3 for package test, (keep existing compat) (#418) Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org> commit 05fe340 Author: st-- <st--@users.noreply.github.com> Date: Tue Dec 21 00:49:37 2021 +0200 use only() instead of first() (#403) * use only() instead of first() for 1-"vectors" that were for the benefit of Flux * fix one test that should not have worked as it was * add missing scalar Sinus constructor commit 2d17212 Author: st-- <st--@users.noreply.github.com> Date: Sat Dec 18 23:43:30 2021 +0200 Zygote AD failure workarounds & test cleanup (#414) Zygote AD failures: * revert #409 (test_utils workaround for broken Zygote - now working again) * disable broken Zygote AD test for ChainTransform Improved tests: * finer-grained testsets * add missing test cases to test_AD * replace test_FiniteDiff with test_AD(..., :FiniteDiff, ...) * remove code duplication commit 3c49949 Author: Théo Galy-Fajou <theo.galyfajou@gmail.com> Date: Wed Nov 24 18:32:19 2021 +0100 Fix typo in valid_inputs error (#408) * Fix typo in valid_inputs error * Update src/utils.jl Co-authored-by: David Widmann <devmotion@users.noreply.github.com> Co-authored-by: David Widmann <devmotion@users.noreply.github.com> commit 9955044 Author: st-- <st--@users.noreply.github.com> Date: Wed Nov 24 18:55:18 2021 +0200 Fix for Zygote 0.6.30 breaking our tests (#409) * restrict Zygote to <0.6.30 * revert Zygote test restriction and add finer-grained testset * Update test/utils.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * revert testset * mark test_broken * Use `@test_throws` instead of `@test_broken` Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: David Widmann <devmotion@users.noreply.github.com> commit 33d64d1 Author: Théo Galy-Fajou <theo.galyfajou@gmail.com> Date: Thu Nov 4 14:23:57 2021 +0100 Add benchmarking CI (#399) * Add benchmark file * delete old benchmarks * Add github action * Add Project * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Missing end of line Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> commit 360ce10 Author: David Widmann <devmotion@users.noreply.github.com> Date: Tue Nov 2 11:09:58 2021 +0100 Update docstring of `GibbsKernel` (#395) * change title * Update manifest * Formatter * Some small updates * Add BenchmarkTools * Move Benchmarks to the correct manifest * Formatter * Fix missed comment * Add ParameterHandling too * Change gif embed * Forgot # * Formatter Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Compat and gif kwargs * , for ; * different commit msg * Some more clarification * Change display * Final changes * Apply suggestions from code review Co-authored-by: st-- <st--@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: st-- <st--@users.noreply.github.com> * Address comments * Missed one * Manifest issue * Apply suggestions from formatter Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Local formatter * Delete loss description. Co-authored-by: st-- <st--@users.noreply.github.com> * format Co-authored-by: ST John <st--@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

restrict Zygote to <0.6.30

9f0c407

This was referenced Nov 19, 2021

**BREAKING** Type Stability for MOInputs #405

Closed

Fix typo in valid_inputs error #408

Merged

willtebbutt approved these changes Nov 19, 2021

View reviewed changes

revert Zygote test restriction and add finer-grained testset

bc53478

github-actions bot reviewed Nov 19, 2021

View reviewed changes

test/utils.jl Outdated Show resolved Hide resolved

st-- changed the title ~~restrict Zygote to <0.6.30~~ Fix for Zygote 0.6.30 breaking our tests Nov 19, 2021

st-- and others added 3 commits November 19, 2021 16:36

Update test/utils.jl

a63445b

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

revert testset

153aa5f

mark test_broken

fa820f4

st-- mentioned this pull request Nov 22, 2021

Zygote 0.6.30 broke @inferred test in check_zygote_type_stability #410

Closed

devmotion reviewed Nov 24, 2021

View reviewed changes

test/test_utils.jl Outdated Show resolved Hide resolved

Use @test_throws instead of @test_broken

b135c7a

theogf approved these changes Nov 24, 2021

View reviewed changes

theogf merged commit 9955044 into master Nov 24, 2021

theogf deleted the st/zygote_issue branch November 24, 2021 16:55

st-- mentioned this pull request Dec 9, 2021

Zygote AD failure workarounds & test cleanup #414

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix for Zygote 0.6.30 breaking our tests #409

Fix for Zygote 0.6.30 breaking our tests #409

Uh oh!

st-- commented Nov 19, 2021

Uh oh!

willtebbutt left a comment •

edited

Loading

Uh oh!

theogf commented Nov 19, 2021

Uh oh!

codecov bot commented Nov 19, 2021 •

edited

Loading

Uh oh!

devmotion commented Nov 19, 2021

Uh oh!

devmotion commented Nov 19, 2021

Uh oh!

Uh oh!

st-- commented Nov 22, 2021

Uh oh!

devmotion commented Nov 22, 2021

Uh oh!

theogf commented Nov 24, 2021

Uh oh!

Uh oh!

st-- commented Nov 25, 2021

Uh oh!

st-- commented Nov 25, 2021

Uh oh!

Uh oh!

Fix for Zygote 0.6.30 breaking our tests #409

Fix for Zygote 0.6.30 breaking our tests #409

Uh oh!

Conversation

st-- commented Nov 19, 2021

Uh oh!

willtebbutt left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theogf commented Nov 19, 2021

Uh oh!

codecov bot commented Nov 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

devmotion commented Nov 19, 2021

Uh oh!

devmotion commented Nov 19, 2021

Uh oh!

Uh oh!

st-- commented Nov 22, 2021

Uh oh!

devmotion commented Nov 22, 2021

Uh oh!

theogf commented Nov 24, 2021

Uh oh!

Uh oh!

st-- commented Nov 25, 2021

Uh oh!

st-- commented Nov 25, 2021

Uh oh!

Uh oh!

willtebbutt left a comment •

edited

Loading

codecov bot commented Nov 19, 2021 •

edited

Loading