-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add the capability to check whether a B-series is energy-preserving #117
Conversation
Thanks, @Sondar74 ! Could you edit the box above (where it says "no description provided") and add a description of the contents of this PR? Specifically:
|
Also, it would be great to make a notebook like https://github.com/ranocha/BSeries.jl/blob/main/docs/src/tutorials/bseries_basics.ipynb but explaining and demonstrating the functionality of the code in this PR. From the Jupyter notebook we can generate a page for the online docs that will look similar to https://ranocha.de/BSeries.jl/dev/tutorials/bseries_basics/. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! As @ketch noted, we have a bit of work to do before we can get this into BSeries.jl. Some general comments (besides the aspects already mentioned by @ketch)
- Please follow standard Julia style, e.g.,
snake_case
for functions and variables. See https://docs.julialang.org/en/v1/manual/style-guide/ - Please add comments explaining why you do something and not only what you do
- Please add references to the basic algorithms that you apply
- Please add docstrings
The Style Guide site recommends not to use the from x -> f(x). Should I rewrite this lines? |
No, that's fine as it is |
…gy_Preserving', translated comments into English, erased two packages, changed the name of some variables and flags.
I haven't looked at the algorithm in details yet. How does it compare to the approach of Sundklakk (2015)? |
…ed the name of 'adjoint()' for 'rightmost_energy_preserving_tree()'
… for the code of the CSRK project.
…e only function taken from 'Combinatorics' is 'permutations'.
I was trying to download the paper from this website, is this the reference you mentioned? And also, can you share the paper with me? |
Done on Slack |
…ble 'signal' from the main functions. Renamed the function 'IsEnergyPreserving' for 'energy_preserving_trees_test'
…ded the lines 'include("Energy_Preserving.jl")', 'using .Energy_Preserving', and 'export OrderMethod'
…hat this is only one function now.
…tion 'is_energy_preserving' is really fast for order 10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! The current version is already much more efficient 👍
Next, there are still some type instabilities and further possibilities for improvements. You can get an impression of the overall performance profile via
julia> using ProfileView, BSeries
julia> series = bseries(10) do t, series
if order(t) in (0, 1)
return 1 // 1
else
v = 1 // 1
n = 0
for subtree in SubtreeIterator(t)
v *= series[subtree]
n += 1
end
return v / (n + 1)
end
end
julia> @profview is_energy_preserving(series) # ignore first profile including compilation
julia> @profview is_energy_preserving(series) # use second profile after compilation
This yields
for me. Some takeaways are
- A substantial amount of time (roughly 50%) is spent in
modified_equation
- which is fine. - You can see that there are type instabilities - the red bars. They are for example cause by lines such as
same_order_trees = []
andsame_order_coeffs = []
, where you createVector{Any}
, fill them, and pass them to another function. Something like this should be avoided by specifying the types concretely (based on the coefficients you get). - You use dense linear algebra. I think it may be worth a try to use sparse linear algebra instead. Then, you should also avoid collecting all the column vectors by just collecting the relevant values and indices while assembling the sparse matrix.
- You call
rank
twice. Depending on whether this is a bottleneck at the end, I wonder whether there is a clever way to avoid this using the SVD or something similar directly.
I will make some fixes to this PR and perform a final review before merging the first working version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
…to Energy_Preserving
This code is based on the Theorem 2 of the paper "Energy-Preserving Integrators and the Structure of B-series" (link: https://link.springer.com/article/10.1007/s10208-010-9073-1).
In this code we want to know if a method is Energy-Preserving or not up to some order.
The function 'EnergyPreserving(A,b,s)' receives as input a square Matrix which is the inner part of the Butcher Array of a numerical method, a vector b (which length is the same as the amount of rows of A) and an intenger s, which will be the order up to which we are testing this method.
For example, the classical Runge-Kutta method:
the function 'EnergyPreserving(A,b,s)' will return 'True' for s = 3,4 but will return 'False' for s = 5, i.e. 'EnergyPreserving(A,b,5)'.
Furthermore, for any method described by the square matrix A and vector b, the function 'OrderMethod' will say up to which order it is energy-preserving. For example, for the same (A,b) as before this function will return "Energy Preserving for order < 4".
Finally, if you have already generated a BSeries
serie
following the next steps:the function
is_energy_preserving(serie)
will returnTrue
fors = 4
, andFalse
fors = 5
.