Skip to content

Conversation

@oxinabox
Copy link
Contributor

I haven't actually looked at the code to see if anything else is needed,
lets see what CI says

@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2021

Codecov Report

Merging #102 (f388eff) into master (787a6a4) will decrease coverage by 0.36%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
- Coverage   75.13%   74.76%   -0.37%     
==========================================
  Files          19       19              
  Lines         543      543              
==========================================
- Hits          408      406       -2     
- Misses        135      137       +2     
Impacted Files Coverage Δ
src/similar_convert_copy.jl 84.09% <0.00%> (-2.28%) ⬇️
src/array_interface.jl 84.00% <0.00%> (-0.80%) ⬇️

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 787a6a4...f388eff. Read the comment docs.

@DhairyaLGandhi
Copy link
Member

This fails Flux tests after upgrading to ChainRules 1.0, so we definitely have fixes needing to go in here. https://buildkite.com/julialang/flux-dot-jl/builds/1496#7bb3aa82-4fcd-4b7f-a3c9-e3de7006c4fa

@DhairyaLGandhi
Copy link
Member

Actually, Flux tests are pulling in ComponentArrays@0.10.5 instead of 0.11, so the compat entry and a release would be great @jonniedie

@DhairyaLGandhi
Copy link
Member

Also tests seem to be pulling an old version of Zygote in CI (v0.4.20 vs v0.6.19). Might be that other compat bounds need to be set.

@DhairyaLGandhi
Copy link
Member

Could we re run ci here?

@jonniedie
Copy link
Collaborator

To be honest, I'm not really sure how to troubleshoot this. Running the tests locally in a fresh temp environment doesn't run into any of these problems. I also can't reproduce the Zygote @v0.4.20 thing, even by forcing all of the versions of the other packages to the versions they show in the CI run.

Any ideas?

@oxinabox
Copy link
Contributor Author

oxinabox commented Aug 4, 2021

we may just need to restart the CI.
It was maybe triggered before everything was registered?

@jonniedie
Copy link
Collaborator

Oh, well it looks like that did it. There's a segfault on nightly, but it doesn't seem to have to do with ComponentArrays. Is that a Zygote thing?

@DhairyaLGandhi
Copy link
Member

I've been seeing it in many places but not sure where it's coming from.

@jonniedie
Copy link
Collaborator

Okay cool. Well I'll go ahead and merge this then. Nightly's gonna nightly.

@jonniedie jonniedie merged commit 1c573f8 into SciML:master Aug 4, 2021
@jonniedie
Copy link
Collaborator

Thanks for doing this @oxinabox. Sorry it took so long for me to get around to merging it.

@oxinabox
Copy link
Contributor Author

oxinabox commented Aug 4, 2021

no problem.

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.

4 participants