Opened 11 months ago

Closed 9 months ago

#33110 closed enhancement (fixed)

Some performance improvements in the manifolds code

Reported by: Marius Gerbershagen Owned by:
Priority: minor Milestone: sage-9.6
Component: manifolds Keywords:
Cc: Merged in:
Authors: Marius Gerbershagen Reviewers: Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: 59691a8 (Commits, GitHub, GitLab) Commit: 59691a84894a18e42341380d0db6012f02348d49
Dependencies: Stopgaps:

Status badges

Description

Changes include:

  • use the first Bianchi identity in the computation of the Riemann tensor
  • compute volume forms with contravariant indices only as needed
  • don't try to simplify trivial expressions consisting only of a single number or symbolic variable

Change History (13)

comment:1 Changed 11 months ago by Marius Gerbershagen

Status: newneeds_review

comment:2 Changed 11 months ago by Eric Gourgoulhon

Thanks for this ticket! I've added it to the metaticket #30525 (fill free to do it yourself for a next ticket related to manifolds) and will take a look in the coming days.

comment:3 Changed 11 months ago by Matthias Köppe

Milestone: sage-9.5sage-9.6

comment:4 Changed 11 months ago by git

Commit: 3f0eec09b49e79c3022475ed2b08fe46a945898259691a84894a18e42341380d0db6012f02348d49

Branch pushed to git repo; I updated commit sha1. New commits:

b53b570Merge branch 'public/manifolds-performance-improvements' of git://trac.sagemath.org/sage into Sage 9.5.rc0
59691a8#33110: reviewer tweak

comment:5 Changed 11 months ago by Eric Gourgoulhon

Reviewers: Eric Gourgoulhon

This is very nice! Thanks a lot! I've just performed some minor tweaks (PEP 8 + adding you to AUTHOR fields) and pushed them in the above commit.

Waiting for the patchbot green light for positive review.

comment:6 Changed 11 months ago by Eric Gourgoulhon

Regarding the computation of the Riemann tensor, I've performed some benchmarks with Kerr, Kerr-Newmann and 5D Kerr-AdS metrics, via these notebooks: 1, 2 and 3 respectively. I've have not noticed any significant speed-up. On your side, do you have any example with some net speed-up?

comment:7 in reply to:  6 Changed 11 months ago by Eric Gourgoulhon

On the other side, I've noticed a tremendous speed-up in the vector field plots, due to the treatment of expressions with a single term in simplifying functions. This is very welcome, until vector field plots are improved by using fast callable expressions (this will require a full rewriting of VectorField.plot, to make it not depend on TangentVector.plot, in some future ticket).

comment:8 in reply to:  6 ; Changed 11 months ago by Marius Gerbershagen

Replying to egourgoulhon:

Regarding the computation of the Riemann tensor, I've performed some benchmarks with Kerr, Kerr-Newmann and 5D Kerr-AdS metrics, via these notebooks: 1, 2 and 3 respectively. I've have not noticed any significant speed-up. On your side, do you have any example with some net speed-up?

I find a small speedup (about 3%) for the Kerr metric in the single-threaded case. The multi-threaded case is a little bit slower for me, but that is most certainly because some of the cores are inactive for a little longer due to the naive way that sage distributes the computation across multiple cores. One should really measure CPU usage instead of total time spent.

But in general my changes reduce the number of components that have to be calculated using contractions of the connection from n2(n-1)/2 to n(n2-1)/3 which in the best case for large n could be up to 33% faster. Of course, if the computation time is dominated by the final simplification step after the contraction (or subtraction when using the Bianchi-identity) has been performed, then there won't be much of a speedup. I suspect that this is the case in your examples which have quite complicated Riemann tensors.

comment:9 in reply to:  8 Changed 11 months ago by Eric Gourgoulhon

Replying to gh-spaghettisalat:

Thanks for your answer.

Of course, if the computation time is dominated by the final simplification step after the contraction (or subtraction when using the Bianchi-identity) has been performed, then there won't be much of a speedup. I suspect that this is the case in your examples which have quite complicated Riemann tensors.

Yes. Most probably, in the case I've considered the computation time is dominated by the gam_gam term, which involves multiplication of connection coefficients and which is computed for the full range of indices, irrespective of the Bianchi identity.

comment:10 in reply to:  5 Changed 10 months ago by Eric Gourgoulhon

Status: needs_reviewpositive_review

Replying to egourgoulhon:

Waiting for the patchbot green light for positive review.

Well after 2 weeks, the patchbot has not shown up... I am setting the ticket to positive review without waiting any further, since all doctests are passed on my computer and the doc builds (both html and pdf).

comment:11 Changed 10 months ago by Eric Gourgoulhon

Status: positive_reviewneeds_review

The ticket branch has not been merged in Sage 9.6.beta1. Maybe it is missing some patchbot green light. So I'm setting the status back to "needs_review" in order to catch the patchbot's attention.

comment:12 Changed 10 months ago by Travis Scrimshaw

Status: needs_reviewpositive_review

There is some order that the tickets get merged in, but I don't quite understand what that is.

comment:13 Changed 9 months ago by Volker Braun

Branch: public/manifolds-performance-improvements59691a84894a18e42341380d0db6012f02348d49
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.