[feature][Python] Add default reference-element containers to the library.
This is done for dim <= 3 to reduce compile times.
Merge request reports
Activity
@andreas.dedner: Could you take a look? It's not pretty but it works without changing much of the workflow.
added 1 commit
- 8612d4aa - [feature][Python] Add default reference containers to the library for
@andreas.dedner: Any objections of this being merged?
@andreas.dedner: Trust me, it's working :-D
added 3 commits
-
8612d4aa...4146f39d - 2 commits from branch
master
- 9dfa53c0 - [feature][Python] Add default reference containers to the library for
-
8612d4aa...4146f39d - 2 commits from branch
added 21 commits
-
9dfa53c0...1a7dddcf - 20 commits from branch
master
- 4c65d01a - [feature][Python] Add default reference containers to the library for
-
9dfa53c0...1a7dddcf - 20 commits from branch
@samuel.burbulla, @andreas.dedner: This MR pre-compiles the reference modules for the common reference elements, i.e. dim <= 3. I think it is a good idea. It is certainly a speedup and the usage does not change. I works with the pip installed version. So everything seems fine. Any thoughts or objections?
mentioned in merge request dune-common!942 (closed)
See also dune-common!942 (closed).
I don't really think this is necessary - I would rather get rid of those libraries all together because they cause problems with shared/dynamic library choices and the rpath. Also, the advantage is really not clear to me, especially in this case. How much time are you saving here? The reference elements compile really fast on my machine especially compared to the real elephant in the room - the compile time of the grid itself.
I didn't know about the shared/dynamic library choices and the rpath problem. I also don't like in this approach here that it represents a mixed approach. There are several benefits though, the module loading becomes instant (no hash necessary) and for the grids the same approach is possible. A branch in alugrid that demonstrates this feature exists already. In general for things that only exists once (like RefElem(dim) we should not employ the complicated create code, compile, load machinery. This makes the code slow. Also, when things are compile as default modules one can use the -jcores option to speed things up. This is not really possible with the current approach (would be nice though).
One could think about putting the current _dune-module.cc as pre-compiled modules in dune-py that are compiled when dune-py is first setup.
Edited by Robert KI've thought about ways to move those libraries into dune-py but I wouldn't want the setup of dune-py be a major computational task either. If you allow any module to add 'predefined' targets to dune-py (which I'm sure is possible) then one would be compiling e.g. alugrid each time dune-py is setup just because one has a dune-alugrid module, i.e., independent of actually wanting to use it.
I would also have to convince myself that the runtime advantage is really that great, e.g., with respect to forming the hash - I would have to check. Being able to parallel build in dune-py would be nice but I don't see any way of doing that which would really bring much advantage. I tested a few things some time ago but couldn't get it to work, i.e., the additional complexity of the approach was never worth the observed gain.
Btw: the issue with the static libraries is that these libraries are compiled in the build directory and then moved into the site-package. They will for example depend on libdunecommon.so for example which is again moved somewhere else when installed. There might be a solution for this but we haven't found it yet which is the reason why everything is done with static libraries at the moment, e.g., in the pip installation.
added 1 commit
- f52ec03c - [feature][Python] Add default reference containers to the library for
added 38 commits
-
f52ec03c...86a72a03 - 37 commits from branch
master
- 97e76c03 - [feature][Python] Add default reference containers to the library for
-
f52ec03c...86a72a03 - 37 commits from branch
mentioned in merge request !185 (closed)
I like this idea a lot and I think this really improves compilation time and usability of the python bindings.
The similar thing that I proposed in !185 (closed) can be dropped in favor of @robert.kloefkorn's implementation. I would like to merge this after merging the cmake-overhaul branches.
Having separate modules for each reference element can be exploited by parallel compilation (which is the default for installed packages because of the Ninja backend). So I would not prefer putting all of them into a single lib.
added 5 commits
-
97e76c03...d158cbb5 - 4 commits from branch
master
- 82d99733 - [feature][Python] Add default reference containers to the library for
-
97e76c03...d158cbb5 - 4 commits from branch
added python label
added 10 commits
-
82d99733...afa1aea7 - 9 commits from branch
master
- 417b5f1a - [feature][Python] Add default reference containers to the library for
-
82d99733...afa1aea7 - 9 commits from branch
In this case the precompile approach is okay with me, although I'm still not too convinced of the advantages.
I did some quick tests (each time first running
make clean
):-
make
:- master 13.41user 1.42system 0:14.20elapsed 104%CPU
- new 73.95user 5.71system 1:17.71elapsed 102%CPU
-
make -j8
:- master 13.70user 1.28system 0:13.33elapsed 112%CPU
- new 74.63user 4.83system 0:22.48elapsed 353%CPU
So if one uses parallel make there is a bit of increase in the make time for
dune-geometry
- without using-j
the increase is considerable. In this case I think it's not an issue but we have to be careful with this approach to not increase module configure time too much.
-
added 88 commits
-
417b5f1a...28241018 - 87 commits from branch
master
- 0d89de64 - [feature][Python] Add default reference containers to the library for
-
417b5f1a...28241018 - 87 commits from branch