Opened 2 years ago

Closed 22 months 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) Commit: 2ba926778bf1fdc57229d260d07b2c449c478b0a
Dependencies: Stopgaps:

Description (last modified by slelievre)

  1. Computation of tensor components in a new frame

In FreeModuleTensor.components()
loop for ind_new in new_comp.non_redundant_index_generator():
line 1101, file src/sage/tensor/modules/free_module_tensor.py

  1. Pullback of a tensor field

In DiffMap.pullback()
loop for ind_new in ptcomp.non_redundant_index_generator():
line 980, file src/sage/manifolds/differentiable/diff_map.py

Change History (28)

comment:1 Changed 2 years ago by mmancini

  • Branch set to public/26168-adding_parallelism_2_functions
  • Commit set to 8b4f9a08855bbc92f09fcc7de15c86459d84ede6

comment:2 Changed 2 years ago by mmancini

  • Description modified (diff)

comment:3 Changed 2 years ago by mmancini

  • Description modified (diff)

comment:4 Changed 2 years ago by mmancini

  • Description modified (diff)

comment:5 Changed 2 years ago by git

  • Commit changed from 8b4f9a08855bbc92f09fcc7de15c86459d84ede6 to 8a808dbc5db2a8859f763c4dc6ba679ceddc7fe3

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

comment:6 Changed 2 years ago by git

  • 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 2 years ago by slelievre

  • 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 22 months ago by git

  • Commit changed from 84c61a7cd2c00e8fe052ec88a8489fd3b59d5d3f to 5f8eaa62c979e058ea79f953ecafd8d44a61a8de

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

c89779bMerge branch 'public/26168-adding_parallelism_2_functions' of git://trac.sagemath.org/sage into develop
5f8eaa6Paralleized also loop on new_comp.non_redundant_index_generator(): in FreeModuleTensor.components()

comment:9 Changed 22 months ago by mmancini

  • Status changed from new to needs_review

Please, the loop functions in the title are parallelized. Please review this ticket.

comment:10 Changed 22 months ago by slelievre

For one instance of Parallel computation::, a blank line is missing after it.

comment:11 Changed 22 months ago by chapoton

Doc does not build

missing empty line here:

+        Parallel computation::
+           sage: Parallelism().set('tensor', nproc=2)

comment:12 follow-up: Changed 22 months ago by git

  • Commit changed from 5f8eaa62c979e058ea79f953ecafd8d44a61a8de to a30055a8aba9b30826a61031da1c50267b4f39bd

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

a30055aDoc corrected

comment:13 in reply to: ↑ 12 ; follow-up: Changed 22 months ago by egourgoulhon

Replying to git:

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

a30055aDoc 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 22 months ago by git

  • Commit changed from a30055a8aba9b30826a61031da1c50267b4f39bd to 2ec62c83490ba5268ef4bdd526121f8223f2cefa

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

2ec62c8deleted import of unused module

comment:15 in reply to: ↑ 13 Changed 22 months ago by mmancini

Replying to egourgoulhon:

import was deleted , thanks

Replying to git:

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

a30055aDoc 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: Changed 22 months ago by egourgoulhon

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 22 months ago by egourgoulhon

  • Status changed from needs_review to needs_work

comment:18 Changed 22 months ago by git

  • Commit changed from 2ec62c83490ba5268ef4bdd526121f8223f2cefa to d1fb9d50e15045de359a70a28ef373f1bdb25c3e

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

d1fb9d5Error in parallel part: corrected. DocTest improved

comment:19 Changed 22 months ago by mmancini

  • Status changed from needs_work to needs_review

comment:20 in reply to: ↑ 16 ; follow-up: Changed 22 months ago by mmancini

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 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:21 in reply to: ↑ 20 ; follow-up: Changed 22 months ago by 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 in display().

Could you remove that doctest?

comment:22 Changed 22 months ago by git

  • Commit changed from d1fb9d50e15045de359a70a28ef373f1bdb25c3e to 2ba926778bf1fdc57229d260d07b2c449c478b0a

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

2ba9267removed ridundant test

comment:23 in reply to: ↑ 21 ; follow-up: Changed 22 months ago by mmancini

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 in display().

Could you remove that doctest?

Done.

comment:24 in reply to: ↑ 23 ; follow-up: Changed 22 months ago by egourgoulhon

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 22 months ago by egourgoulhon

  • Reviewers set to Frédéric Chapoton, Eric Gourgoulhon, Samuel Lelièvre
  • Status changed from needs_review to positive_review

comment:26 Changed 22 months ago by egourgoulhon

  • Milestone changed from sage-8.4 to sage-8.5

comment:27 in reply to: ↑ 24 Changed 22 months ago by mmancini

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 22 months ago by vbraun

  • Branch changed from public/26168-adding_parallelism_2_functions to 2ba926778bf1fdc57229d260d07b2c449c478b0a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.