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

Speedup needed in higher_order_symbolics! #18

Open
rusandris opened this issue Apr 3, 2024 · 4 comments
Open

Speedup needed in higher_order_symbolics! #18

rusandris opened this issue Apr 3, 2024 · 4 comments

Comments

@rusandris
Copy link
Owner

rusandris commented Apr 3, 2024

When dealing with higher order states, a significant amount of time is spent on transforming to states of higher order.

using DynamicalSystems,StateTransitionNetworks
ds = Systems.logistic()
orbit,t = trajectory(ds,10^8;Ttr=1000)
sts = timeseries_to_grid(orbit,20)

@time higher_order_symbolics!(sts,10)
 13.534721 seconds (106.11 k allocations: 7.337 MiB, 1.12% compilation time)

We can speed it up by remapping the original symbols before doing anything else (even though this means we are going to loop through symbolic_timeseries twice) :

 @time higher_order_symbolics!(sts,10)
  6.721250 seconds (87.98 k allocations: 6.084 MiB, 2.22% compilation time))

See fix here ba0db6c

@rusandris
Copy link
Owner Author

rusandris commented Apr 3, 2024

Readibility could also be improved by wrapping the loops into separate functions

@rusandris
Copy link
Owner Author

TODO: add docstring, and don't forget to mention that the last order-1 elements aren't converted to higher order states

@rusandris
Copy link
Owner Author

If we allow calculate_transition_matrix to do the remapping in-place beforehand, it can be omitted entirely from higher_order_symbolics!.

And calling higher_order_state can be avoided since loops can be merged into one.
See this here 5f279df , also 28570fa

With all these we get even better performance:

@time higher_order_symbolics!(sts,10)
  4.801914 seconds (26.55 k allocations: 1.884 MiB, 1.25% compilation time)

@rusandris
Copy link
Owner Author

Option needed in case one needs remapping (calculate_transition_matrix) wasn't run before resorting to higher order states

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

No branches or pull requests

1 participant