Opened 4 years ago
Closed 4 years ago
#26168 closed enhancement (fixed)
Add parallelism for two tensor calculus functions
Reported by: | mmancini | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.5 |
Component: | performance | Keywords: | tensor |
Cc: | egourgoulhon, slelievre | Merged in: | |
Authors: | Marco Mancini | Reviewers: | Frédéric Chapoton, Eric Gourgoulhon, Samuel Lelièvre |
Report Upstream: | N/A | Work issues: | |
Branch: | 2ba9267 (Commits, GitHub, GitLab) | Commit: | 2ba926778bf1fdc57229d260d07b2c449c478b0a |
Dependencies: | Stopgaps: |
Description (last modified by )
- Computation of tensor components in a new frame
In
FreeModuleTensor.components()
loopfor ind_new in new_comp.non_redundant_index_generator():
line 1101, filesrc/sage/tensor/modules/free_module_tensor.py
- Pullback of a tensor field
In
DiffMap.pullback()
loopfor ind_new in ptcomp.non_redundant_index_generator():
line 980, filesrc/sage/manifolds/differentiable/diff_map.py
Change History (28)
comment:1 Changed 4 years ago by
- Branch set to public/26168-adding_parallelism_2_functions
- Commit set to 8b4f9a08855bbc92f09fcc7de15c86459d84ede6
comment:2 Changed 4 years ago by
- Description modified (diff)
comment:3 Changed 4 years ago by
- Description modified (diff)
comment:4 Changed 4 years ago by
- Description modified (diff)
comment:5 Changed 4 years ago by
- Commit changed from 8b4f9a08855bbc92f09fcc7de15c86459d84ede6 to 8a808dbc5db2a8859f763c4dc6ba679ceddc7fe3
comment:6 Changed 4 years ago by
- Commit changed from 8a808dbc5db2a8859f763c4dc6ba679ceddc7fe3 to 84c61a7cd2c00e8fe052ec88a8489fd3b59d5d3f
Branch pushed to git repo; I updated commit sha1. New commits:
84c61a7 | _pullback_chart function in class DiffMap parallelized
|
comment:7 Changed 4 years ago by
- Cc slelievre added
- Description modified (diff)
- Summary changed from adding parallelism for two functions (tensor calculus) to Add parallelism for two tensor calculus functions
comment:8 Changed 4 years ago by
- Commit changed from 84c61a7cd2c00e8fe052ec88a8489fd3b59d5d3f to 5f8eaa62c979e058ea79f953ecafd8d44a61a8de
comment:9 Changed 4 years ago by
- Status changed from new to needs_review
Please, the loop functions in the title are parallelized. Please review this ticket.
comment:10 Changed 4 years ago by
For one instance of Parallel computation::
, a blank line is missing after it.
comment:11 Changed 4 years ago by
Doc does not build
missing empty line here:
+ Parallel computation:: + sage: Parallelism().set('tensor', nproc=2)
comment:12 follow-up: ↓ 13 Changed 4 years ago by
- Commit changed from 5f8eaa62c979e058ea79f953ecafd8d44a61a8de to a30055a8aba9b30826a61031da1c50267b4f39bd
Branch pushed to git repo; I updated commit sha1. New commits:
a30055a | Doc corrected
|
comment:13 in reply to: ↑ 12 ; follow-up: ↓ 15 Changed 4 years ago by
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
a30055a Doc corrected
Could you please also correct the pyflakes error reported in https://patchbot.sagemath.org/log/26168/Gentoo%20Base%20System/2.2/x86_64/4.4.26-gentoo/sage4/2018-11-20%2013:42:02?plugin=pyflakes, namely
src/sage/manifolds/differentiable/diff_map.py:37: 'operator.itemgetter' imported but unused
comment:14 Changed 4 years ago by
- Commit changed from a30055a8aba9b30826a61031da1c50267b4f39bd to 2ec62c83490ba5268ef4bdd526121f8223f2cefa
Branch pushed to git repo; I updated commit sha1. New commits:
2ec62c8 | deleted import of unused module
|
comment:15 in reply to: ↑ 13 Changed 4 years ago by
Replying to egourgoulhon:
import was deleted , thanks
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
a30055a Doc corrected
Could you please also correct the pyflakes error reported in https://patchbot.sagemath.org/log/26168/Gentoo%20Base%20System/2.2/x86_64/4.4.26-gentoo/sage4/2018-11-20%2013:42:02?plugin=pyflakes, namely
src/sage/manifolds/differentiable/diff_map.py:37: 'operator.itemgetter' imported but unused
comment:16 follow-up: ↓ 20 Changed 4 years ago by
The doctests regarding the change of basis in src/sage/tensor/modules/free_module_tensor.py
are not valid because the tensor components are cached. One must introduce a copy of the tensor t
to trigger a new computation of the components in the basis f
, i.e. one shall write something like
sage: Parallelism().set('tensor', nproc=2) sage: t2 = v*w # identical to t but initialized only in basis e sage: t2.display(f) v*w = -6 f_1*f^1 - 20/3 f_1*f^2 + 27/8 f_2*f^1 + 15/4 f_2*f^2 sage: t2[f,:] == t[f,:] # check of the parallel computation True sage: Parallelism().set('tensor', nproc=1) # switch off parallelization
But then t2.display(f)
results in
ValueError: Cannot pickle code objects from closures
The doctest in display_comp()
is also not valid, but I suggest to simply remove it because it is redundant with that in display()
.
comment:17 Changed 4 years ago by
- Status changed from needs_review to needs_work
comment:18 Changed 4 years ago by
- Commit changed from 2ec62c83490ba5268ef4bdd526121f8223f2cefa to d1fb9d50e15045de359a70a28ef373f1bdb25c3e
Branch pushed to git repo; I updated commit sha1. New commits:
d1fb9d5 | Error in parallel part: corrected. DocTest improved
|
comment:19 Changed 4 years ago by
- Status changed from needs_work to needs_review
comment:20 in reply to: ↑ 16 ; follow-up: ↓ 21 Changed 4 years ago by
Replying to egourgoulhon: There was an error in the parallel function: it is corrected. I also applied the suggested changes in the doctest thank
The doctests regarding the change of basis in
src/sage/tensor/modules/free_module_tensor.py
are not valid because the tensor components are cached. One must introduce a copy of the tensort
to trigger a new computation of the components in the basisf
, i.e. one shall write something likesage: Parallelism().set('tensor', nproc=2) sage: t2 = v*w # identical to t but initialized only in basis e sage: t2.display(f) v*w = -6 f_1*f^1 - 20/3 f_1*f^2 + 27/8 f_2*f^1 + 15/4 f_2*f^2 sage: t2[f,:] == t[f,:] # check of the parallel computation True sage: Parallelism().set('tensor', nproc=1) # switch off parallelizationBut then
t2.display(f)
results inValueError: Cannot pickle code objects from closuresThe doctest in
display_comp()
is also not valid, but I suggest to simply remove it because it is redundant with that indisplay()
.
comment:21 in reply to: ↑ 20 ; follow-up: ↓ 23 Changed 4 years ago by
Replying to mmancini:
Replying to egourgoulhon: There was an error in the parallel function: it is corrected. I also applied the suggested changes in the doctest
Thanks.
The doctest in
display_comp()
is also not valid, but I suggest to simply remove it because it is redundant with that indisplay()
.
Could you remove that doctest?
comment:22 Changed 4 years ago by
- Commit changed from d1fb9d50e15045de359a70a28ef373f1bdb25c3e to 2ba926778bf1fdc57229d260d07b2c449c478b0a
Branch pushed to git repo; I updated commit sha1. New commits:
2ba9267 | removed ridundant test
|
comment:23 in reply to: ↑ 21 ; follow-up: ↓ 24 Changed 4 years ago by
Replying to egourgoulhon:
Replying to mmancini:
Replying to egourgoulhon: There was an error in the parallel function: it is corrected. I also applied the suggested changes in the doctest
Thanks.
The doctest in
display_comp()
is also not valid, but I suggest to simply remove it because it is redundant with that indisplay()
.Could you remove that doctest?
Done.
comment:24 in reply to: ↑ 23 ; follow-up: ↓ 27 Changed 4 years ago by
Replying to mmancini:
Could you remove that doctest?
Done.
Thanks. I've tested the changes both in python2 and python3. Everything is OK and I thing it is ready to go. Thanks for this improvement!
comment:25 Changed 4 years ago by
- Reviewers set to Frédéric Chapoton, Eric Gourgoulhon, Samuel Lelièvre
- Status changed from needs_review to positive_review
comment:26 Changed 4 years ago by
- Milestone changed from sage-8.4 to sage-8.5
comment:27 in reply to: ↑ 24 Changed 4 years ago by
Replying to egourgoulhon:
Thanks to you for the review
Replying to mmancini:
Could you remove that doctest?
Done.
Thanks. I've tested the changes both in python2 and python3. Everything is OK and I thing it is ready to go. Thanks for this improvement!
comment:28 Changed 4 years ago by
- Branch changed from public/26168-adding_parallelism_2_functions to 2ba926778bf1fdc57229d260d07b2c449c478b0a
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits: