Description

FeldrinH

opened on Feb 9, 2024

Issue body actions

There seems to be some inconsistency with the ordering of parameters for ODE functions between phase_plot and StateSpace.dynamics. phase_plot expects a function with signature f(x, t), whereas StateSpace.dynamics and other functions that are meant to be used with scipy.integrate.solve_ivp provide a function with the signature f(t, x) (note that the order of arguments is different).

It would be nice if the order of arguments was consistent, so you could pass functions compatible with scipy.integrate.solve_ivp directly to phase_plot.

Activity

1. sawyerbfuller commented on Mar 5, 2024

   

   sawyerbfuller

   on Mar 5, 2024

   Contributor

   More actions

   Yes I agree, phase_plot has the wrong parameter order, leftover from the previous convention. The docstring should also be updated to reference scipy's new integration scheme as well in scipy.integrate.solve_ivp.

   Please submit a pull request!
2. FeldrinH commented on Mar 5, 2024

   

   FeldrinH

   on Mar 5, 2024

   ContributorAuthor

   More actions

   @murrayrm commented in #970 (comment) that he is rewriting parts of phase_plot, so it might not be worthwhile for an external contributor to do a pull request changing the old phase_plot code right now.
3. murrayrm

   linked a pull request that will close this issueReimplementation of 2D phase plots #980on Mar 28, 2024
4. murrayrm

   closed this as completedin #980on Mar 31, 2024

Issue actions