Merge mpi to master. Better with rebase.
Hi,
#68 is about finishing the mpi implementation. Let's use this issue #80 (closed) for merging mpi branch with master.
First Deepak asked about this. (btw: I prefer to write about this here in an issue on gitlab, because it offers nice formatting). I looked at the merge conflicts and I have pushed a first version of merge in this branch. Now I look into this again and I will discuss this merge here as I look through the conflicts. At the end I will push another branch.
EDIT: It is now in branch https://gitlab.com/yade-dev/trunk/tree/mpiRebase and in MR !139 (closed)
rebase
After we finish this merge (in another branch), we will have to rebase master, for two reasons:
- We need
git bisect
to still work after merging mpi to master. with rebase all mpi commits are one after another in the history. Without rebase they are all interleaved, which means that most of the commits will not even compile (because it has some of the changes from master, and some changes from mpi, interleaved). - Without rebase these conflicts may come up again.
tools for solving conflicts
To examine conflicts I recommend fugitive plugin for gvim. It helps a lot. I only type :Gblame, and I see this:
InsertionSortCollider.cpp
So to solve InsertionSortCollider.cpp
I propose:
if(fastestBodyMaxDist>=1 || fastestBodyMaxDist==0) { return true; }
if(BB[0].size() != 2*scene->bodies->size()) { return true; }
if(scene->interactions->dirty) { return true; }
if(scene->doSort) { return true; }
I see that Anton wrote in 2012 scene->doSort=false;
, but Bruno removed it in 2018. So I went along with that.
For next conflict we have:
numAction++;
const size_t nBodies = scene->bodies->size();
Also :Gdiff command is very useful. We can quickly get lines from mpi or master with command :diffget //2 (mpi) or :diffget //3 (master)
main.py.in
OK, in main.py.in
I think it is clear:
if (gui!='none'): yade.qt.Controller();
and
if not opts.mpi_mode: ipshell()
Thermal.?pp
The files pkg/pfv/Thermal.cpp
and pkg/pfv/Thermal.hpp
are conflicted like crazy, so I look at :Gblame, and see that master is mostly from 2018-10-23 while mpi is one month older. So I chose only master with command :Gwrite.
.gitlab-ci.yml
The file .gitlab-ci.yml
I take from master. It will change anyway.
doc/sphinx/*.rst
Files doc/sphinx/installation.rst
and doc/sphinx/github.rst
I also take from master.
CMakeLists.txt
File CMakeLists.txt
I take almost entirely from master, with exception of these lines:
ADD_LIBRARY(boot SHARED ${CMAKE_CURRENT_SOURCE_DIR}/core/main/pyboot.cpp)
SET_TARGET_PROPERTIES(boot PROPERTIES PREFIX "" LINK_FLAGS "-Wl,--as-needed" )
-TARGET_LINK_LIBRARIES(yade ${Boost_LIBRARIES} ${PYTHON_LIBRARIES} ${LINKLIBS} -lrt)
+
+IF(ENABLE_MPI)
+ TARGET_LINK_LIBRARIES(yade ${Boost_LIBRARIES} ${PYTHON_LIBRARIES} ${LINKLIBS} ${MPI_C_LIBRARIES} ${MPI_CXX_LIBRARIES} -lrt)
+ELSE(ENABLE_MPI)
+ TARGET_LINK_LIBRARIES(yade ${Boost_LIBRARIES} ${PYTHON_LIBRARIES} ${LINKLIBS} -lrt)
+ENDIF(ENABLE_MPI)
+
SET_TARGET_PROPERTIES(yade PROPERTIES LINK_FLAGS "-Wl,--as-needed" )
TARGET_LINK_LIBRARIES(boot yade)
I'm not 100% sure, but I think that Bruno mentioned somewhere that yade should be linked to mpi?
But also I suspect that this should be written somewhere else? Presumably add MPI to ${LINKLIBS}
in the MPI section? Also Anton said that ${MPI_C_LIBRARIES} are not necessary?
I think this problem is related to #79 (closed) too.
However all other CMakeLists.txt
changes required for mpi are already in master. Because we have OpenFOAM working :)
So I hope that you can look at this and fix CMakeLists.txt
.
core/main/yade-oar.in
In core/main/yade-oar.in
something weird happened. In master affinity
was added, but in mpi nCpu
was added. I try to merge these two together like this (add both nCpu and affinity):
class JobInfo(object):
def __init__(self,num,id,command,log,nCores,nCpu,script,table,lineNo,affinity,oarprop):
self.started,self.finished,self.duration,self.durationSec,self.exitStatus=None,None,None,None,None # duration is a string, durationSec is a number
self.command=command; self.num=num; self.log=log; self.id=id; self.nCores=nCores; self.nCpu=nCpu; self.cores=set(); self.infoSocket=None
self.script=script; self.table=table; self.lineNo=lineNo; self.affinity=affinity
But it would be much better if @chevremw looked at this. Because he added both of those!
InteractionLoop.cpp
In InteractionLoop.cpp
in the mpi branch you have rediscovered the bug concerning interactions. I have already started working on it as you can see by my comments when working on #48 (closed) and !36 (diffs), I will open a separate issue about that.
FlowBoundingSphere.ipp
I see that master removed tesselation()
everywhere, so I went along with that. Besides that those two conflicted lines seem identical, so I am not sure why they went into conflict in the first place.
FlowBoundingSphereLinSolv.ipp
The first conflict is just a comment, so that's easy. But the other conflict is a contradiction ! I use master for this (because the line is newer by one year), but seriously @robcaulk should look into this - you have written these lines :)
if (!reuseOrdering) {
L = cholmod_l_analyze(Achol, &com);
<<<<<<< HEAD
M = cholmod_l_copy_factor(L, &com);
} else {
N = cholmod_l_copy_factor(M, &com);
=======
} else {
N = cholmod_l_copy_factor(L, &com);
>>>>>>> master
}
This line is from year 2019, so I pick it:
if (!cell->info().isGhost) cell->info().internalEnergy = fluidCp*fluidRho*cell->info().temp()*(1./cell->info().invVoidVolume());
And these lines are newer in master:
RTriangulation& Tri = T[currentTes].Triangulation();
FiniteCellsIterator cellEnd = Tri.finite_cells_end();
for (FiniteCellsIterator cell = Tri.finite_cells_begin(); cell != cellEnd; cell++){
if (!cell->info().Tcondition && !cell->info().isGhost) {
Real oldTemp = cell->info().temp();
cell->info().temp()=cell->info().internalEnergy/((1./cell->info().invVoidVolume())*fluidCp*fluidRho); //FIXME: invVoidVolume depends on volumeSolidPore() which uses CGAL points only updated each remesh. We need our own volumeSolidPore().
cell->info().dtemp() = cell->info().temp() - oldTemp;
}
}
So I use them.
@robcaulk I've put #warning for you in these places.
GlobalStiffnessTimeStepper.cpp
This conflict is strange:
One month later in master Bruno has removed the (not viscEl)
so I go along with that. But put a #warning
VTKRecorder.cpp
I simply chose master here.
FlowEngine.hpp.in
Here I use the most recent commits, and they come from master:
void pyResetLinearSystem() { solver->resetLinearSystem(); }
CELL_SCALAR_GETTER(Real,.invVoidV,invInitVoidVolume)
((bool, iniVoidVolumes, false,,"activate the computation of the inverse of the initial void volumes in each cell when pore volumes are initialized."))
Real tempVariation;
FlowEngine.ipp.in
That one was easy: two functions were added in master. Let's keep them :)
_utils.hpp
Wery bad! You have added mpi function to utils.cpp ! You should create a new python module for that, see an example here d067b069
Apart from that the conflict in .hpp was trivial. Just put this stuff into #ifdef YADE_MPI
, but you should move it into separate file.
_utils.cpp
First and third conflict only had Shop::
missing. So let's keep it.
Second conflict I think is a merge: 2 lines added in mpi branch, 1 line added in master:
py::def("serializeMPIBodyContainer",serializeMPIBodyContainer, (py::arg("container")), "test serializing . ");
py::def("deSerializeMPIBodyContainer",deSerializeMPIBodyContainer,(py::arg("containerString")), "test deserializing");
py::def("initMPI", initMPI, "Initialize MPI communicator, for Foam Coupling");
But how is it possible that mpi branch was working without initMPI
function? It was addedd only for OpenFOAM ?
timing.py
This one is a little crazy. Bruno was already merging it in 2018-06-13, and left some leftovers. Well, OK. I try to clean this up. But if timing.py doesn't work for you, then you better have a look :)
Other files?
The worst part is that maybe some files were merged automatically in an incorrect way. So the best to verify me, is that you try the merge yourself and compare with what I have done.
For example:
FlowEngine.hpp.in
This line:
((double, fluidK, 0.650,,"Thermal conductivity of the fluid (for thermalEngine)."))
was removed when I used git merge master
on mpi branch. But was added when I used git pull --rebase upstream master
.
So @robcaulk please check this! I added a #warning there.
aabbExtrema()
On the other side git merge master
somehow preferred Jerome's version of aabbExtrema()
. But fortunately I noticed this (because git pull --rebase upstream master
wanted the Bruno's version), and I used the version which Bruno fixed 3 months later.
There may be more such things.
I have pushed this as https://gitlab.com/yade-dev/trunk/tree/mpiRebase branch.