#18100 closed enhancement (fixed)
Parallelization of computations on tensors on free modules
Reported by: | egourgoulhon | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.10 |
Component: | linear algebra | Keywords: | parallelization, free module, tensor |
Cc: | tscrim | Merged in: | |
Authors: | Marco Mancini | Reviewers: | Vincent Delecroix |
Report Upstream: | N/A | Work issues: | |
Branch: | 21d1d70 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
This ticket implements parallelization of basic operations (arithmetics, contractions) on tensors on free modules of finite rank. The parallelization is performed by means of Python's module multiprocessing
, via the decorator @parallel
. It is implemented in computational methods of classes handling tensor components: sage.tensor.modules.comp.Components
and sage.tensor.modules.comp.CompWithSym
. The user can control the number of processes involved in the parallelization via the singleton class Parallelism
. The latter, introduced in this ticket, is actually generic and can be used in other parts of Sage.
This work is an extension of #15916, within the SageManifolds project (actually the code in this ticket is the basis for parallelization in SageManifolds 0.8). See the metaticket #18528 for an overview.
Change History (32)
comment:1 Changed 6 years ago by
- Branch set to public/18100-parallelization_tensors
- Commit set to 2f9352531d734bad923eb72bb335af843bfed3c8
comment:2 Changed 6 years ago by
- Commit changed from 2f9352531d734bad923eb72bb335af843bfed3c8 to 208015433bc7538a62c47fb09b27a58def53704d
Branch pushed to git repo; I updated commit sha1. New commits:
2080154 | Renamed class for parallelism in the Singleton class TensorParallelism. Added doc/test for TensorParallelism
|
comment:3 Changed 6 years ago by
- Commit changed from 208015433bc7538a62c47fb09b27a58def53704d to 5659f29c3a1d5a7ae9601cece8198a6e277384fa
Branch pushed to git repo; I updated commit sha1. New commits:
5659f29 | Added parallelization on tensor product
|
comment:4 Changed 6 years ago by
- Commit changed from 5659f29c3a1d5a7ae9601cece8198a6e277384fa to 325e83284ff3b7e0fcb3cf3b6c196814ffb5874e
Branch pushed to git repo; I updated commit sha1. New commits:
325e832 | Corrected error in parallelisation of __mul__. Added parallism for the symmetric __add__
|
comment:5 Changed 6 years ago by
- Commit changed from 325e83284ff3b7e0fcb3cf3b6c196814ffb5874e to 256d26894f461b15acb2be7fb5815fc06cff5405
Branch pushed to git repo; I updated commit sha1. New commits:
256d268 | changed name TensorParallelism to TensorParallelCompute
|
comment:6 Changed 6 years ago by
- Commit changed from 256d26894f461b15acb2be7fb5815fc06cff5405 to bea739dea7a019607b770eafc5e436392e7f0bbf
Branch pushed to git repo; I updated commit sha1. New commits:
bea739d | Added Eric corrections to parallelism
|
comment:7 Changed 6 years ago by
- Commit changed from bea739dea7a019607b770eafc5e436392e7f0bbf to 81f662cc7cb2a1add3e14d862654d1658f16bb43
comment:8 Changed 6 years ago by
- Milestone changed from sage-6.6 to sage-6.8
comment:9 Changed 6 years ago by
- Commit changed from 81f662cc7cb2a1add3e14d862654d1658f16bb43 to ea38c84d685446a1de5793cd00d79ea6c5448605
Branch pushed to git repo; I updated commit sha1. New commits:
ea38c84 | In doc, changed processor with process
|
comment:10 Changed 6 years ago by
- Status changed from new to needs_review
comment:11 Changed 6 years ago by
- Cc tscrim added
- Description modified (diff)
comment:12 Changed 6 years ago by
- Description modified (diff)
comment:13 follow-up: ↓ 14 Changed 6 years ago by
- Status changed from needs_review to needs_info
Hello,
The name set_nproc
is not appropriate for a function that is intended to be used only for tensor products. Secondly, why are you duplicating the interface with a class TensorParallelCompute
and functions set_nproc
, get_nproc
?
Vincent
comment:14 in reply to: ↑ 13 Changed 6 years ago by
Replying to vdelecroix: Hello Vincent,
Thanks to review our work.
We try to reply to the two points:
1) You are right, the names set_nproc
and get_nproc
are generic names and not clearly referred to tensors, so we propose to changes them in set_nproc_tensor
and get_nproc_tensor
.
2) The functions set_nproc
and get_nproc
were introduced in order to avoid
to the users to typing the two more verbose (and less clear) functions TensorParallelCompute().set(nproc)
and TensorParallelCompute._nproc
Note that the singleton TensorParallelCompute
is not in the global namespace but only the functions set_nproc
and get_nproc
.
Best regards,
Marco
Hello,
The name
set_nproc
is not appropriate for a function that is intended to be used only for tensor products. Secondly, why are you duplicating the interface with a classTensorParallelCompute
and functionsset_nproc
,get_nproc
?Vincent
comment:15 Changed 6 years ago by
- Commit changed from ea38c84d685446a1de5793cd00d79ea6c5448605 to 819661d309420c1b76e757ba9335061e7a2001f9
Branch pushed to git repo; I updated commit sha1. New commits:
819661d | Changed variables name : set_nproc to set_nproc_tensor, get_nproc to get_nproc_tensor
|
comment:16 Changed 6 years ago by
- Commit changed from 819661d309420c1b76e757ba9335061e7a2001f9 to 53c248cd5f915134eb63e96050b0c5b5f9e47996
Branch pushed to git repo; I updated commit sha1. New commits:
53c248c | Parallelization: corrected doc
|
comment:17 Changed 6 years ago by
In the last 2 commits we modified the variables name set_nproc get_nproc in set_nproc_tensor and get_nproc_tensor as proposed.
comment:18 Changed 6 years ago by
- Status changed from needs_info to needs_review
comment:19 Changed 5 years ago by
- Milestone changed from sage-6.8 to sage-6.10
comment:20 Changed 5 years ago by
- Reviewers set to Vincent Delecroix
- Status changed from needs_review to needs_info
Some more questions:
- I still don't understand why there is an object oriented interface and a function interface. Why not only one?
- Wouldn't it be better to gather everything related to parallelism in a single place? You could mimic what is done for proofs (
sage.structure.proof.proof
) or global options (sage.structure.global_options
). These two are not perfect but at least they intend to be general and reusable. The object oriented interface could be something like:sage: Parallelism().get('tensor') 1 sage: Parallelism().set('tensor', 3) sage: Parallelism().set_default(3) sage: Parallelism().get_default() 3 sage: Parallellism().get_all() { 'tensor': 3, 'xyz': 2 }
(though that could be postponed to some other tickets)
- The functions
get_nproc_tensor
andset_nproc_tensor
should definitely not be in the global namespace as they are very much specialized.
comment:21 Changed 5 years ago by
- Commit changed from 53c248cd5f915134eb63e96050b0c5b5f9e47996 to ade22b1abf6842fac14a3c61da43189d1b0279d4
Branch pushed to git repo; I updated commit sha1. New commits:
ade22b1 | Created Singleton class Parallelism more generic. Persist problem with doc and tests
|
comment:22 Changed 5 years ago by
- Commit changed from ade22b1abf6842fac14a3c61da43189d1b0279d4 to 4e64db3784a823a5d9912024c41f0f53e423ad09
Branch pushed to git repo; I updated commit sha1. New commits:
4e64db3 | Fix doctests and documentation issues in class Parallelism
|
comment:23 follow-up: ↓ 25 Changed 5 years ago by
Thanks for the heavy changes! If you are done with your changes and want a review again you need to switch the status of the ticket from "needs info" to "needs review".
You also need to update the ticket description.
It would be good to advertise this ticket on sage-devel. Other people might be interested (e.g. #18987 for which the parallelization option can use some default).
comment:24 Changed 5 years ago by
- Status changed from needs_info to needs_work
comment:25 in reply to: ↑ 23 Changed 5 years ago by
Replying to vdelecroix:
Thanks for the heavy changes! If you are done with your changes and want a review again you need to switch the status of the ticket from "needs info" to "needs review".
Actually a few improvements in the documentation are under way and will be committed soon.
You also need to update the ticket description.
Yes indeed.
It would be good to advertise this ticket on sage-devel. Other people might be interested (e.g. #18987 for which the parallelization option can use some default).
OK.
comment:26 Changed 5 years ago by
- Commit changed from 4e64db3784a823a5d9912024c41f0f53e423ad09 to b0332975733cdda87b65707273fc42975ba96020
Branch pushed to git repo; I updated commit sha1. New commits:
b033297 | Improvements in the documentation of class Parallelism
|
comment:27 Changed 5 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:28 Changed 5 years ago by
Note that the documentation of class Parallelism
has been added to the Parallel Computing section of the reference manual.
comment:29 Changed 5 years ago by
- Commit changed from b0332975733cdda87b65707273fc42975ba96020 to 21d1d70c1da23187c7d230dd950c45980d13cae6
Branch pushed to git repo; I updated commit sha1. New commits:
21d1d70 | Minor modification in the doctests of parallel computation in class Components
|
comment:30 follow-up: ↓ 32 Changed 5 years ago by
- Status changed from needs_review to positive_review
Hello,
All right. This is a good start for further improvements...
Vincent
comment:31 Changed 5 years ago by
- Branch changed from public/18100-parallelization_tensors to 21d1d70c1da23187c7d230dd950c45980d13cae6
- Resolution set to fixed
- Status changed from positive_review to closed
comment:32 in reply to: ↑ 30 Changed 5 years ago by
- Commit 21d1d70c1da23187c7d230dd950c45980d13cae6 deleted
Replying to vdelecroix:
Hello,
All right. This is a good start for further improvements...
Vincent
Hello Vincent,
Thanks a lot for the review, advices and suggestions. Regards, Marco
New commits:
merged parallelisation from SageManifold
added paralellization