search

SciML / SciMLBase.jl

The Base interface of the SciML ecosystem
https://docs.sciml.ai/SciMLBase/stable
MIT License
119 stars 91 forks source link

fix deprecated indexing in EnsembleSummary plot recipe #604

Closed SebastianM-C closed 5 months ago

SebastianM-C commented 5 months ago

I missed this ones. @AayushSabharwal is this ok? It seems a bit odd to need sim.u.u[1] at first sight.

codecov[bot] commented 5 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (5d3ac43) 39.96% compared to head (d6bb6e2) 39.96%.

Files Patch % Lines
src/ensemble/ensemble_solutions.jl 0.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #604 +/- ## ======================================= Coverage 39.96% 39.96% ======================================= Files 54 54 Lines 4076 4076 ======================================= Hits 1629 1629 Misses 2447 2447 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AayushSabharwal commented 5 months ago

Yeah that is weird. VectorOfArrays shouldn't have a VectorOfArray as u. I'll investigate this further

SebastianM-C commented 5 months ago

I'm not sure if it helps, but I was wondering if this is due to the fact that individual ODESolutions are no longer AbstractArrays and the recipes relied on this to distinguish between scalar and vector cases, as for ensemble you can have scalar differential equations or output functions that return scalars.

AayushSabharwal commented 5 months ago

This does seem to be what is required here. I'm not fully on board with the AbstractVectorOfArray.u being another AbstractVectorOfArray thing that's going on, but if it works it works I guess.

SebastianM-C commented 5 months ago

Should this be merged?

cc @ChrisRackauckas