Add nompi stub

[bobpaw]

Add nompi stub

Add a stub implementation of MPI in pcu/pcu_pnompi.c.
Remove direct calls to MPI functions.

This adds a SCOREC_NO_MPI build option for users running on single-process, single-machine setups that do not want to compile with MPI. Message passing is simulated in memory.

[bobpaw]

@cwsmith, build is failing for this PR. I think we should add PCU_Init and PCU_Finalize facades that wrap the MPI calls and do a PCU_Comm_Init() and PCU_Comm_Free() to remove the direct dependencies. There are a lot of executables that would need to be changed and this would become a large PR. Alternatively, we could define a replacement MPI_Init and MPI_Finalize in pcu_pnompi and link to those. Or, we could do a hybrid approach and encourage people to use PCU_Init in the future.

[jacobmerson]

@flagdanger and I are working on a very large update of PCU that should make this all easier by getting rid of the singleton. Pull request for that coming within the next week.

[jacobmerson]

@bobpaw sorting out a few things to make the pull request ready to merge, but #388 the draft pull request for replacing the singleton in PCU.

Although we replaced the interface used in the test cases, this pull request is designed to work with no modification to existing codes by using a global PCU state when the old style interface is used with PCU_Comm_Init or it can use local state.

[bobpaw]

@jacobmerson, we will plan to use your PR for the future of core, but the sponsor wants this functionality soon, so we will use this branch until yours is finished.

[cwsmith]

@bobpaw Does PCU_Comm_Init() check if MPI was initialized?

[bobpaw]

@bobpaw Does PCU_Comm_Init() check if MPI was initialized?

No, at the moment it only checks if PCU has been initialized using a global state variable. We could check with MPI_Initialized.

[onkarsahni]

@cwsmith Note Aiden is only doing this when SCOREC_NO_MPI is defined to be true, so there is no actual MPI (e.g., see pcu/pcu_pnompi.c). For now, he is not looking into PCU_init, that will be done after the other PR #388.

[cwsmith]

@bobpaw OK. I'm wondering if it will make sense to make the MPI check in that call, and if it fails call MPI_Init. But... handling MPI_Finalize could be a bit more tricky; other libraries may still be running and using MPI beyond the lifetime of PCU. I'd prefer symmetry in the interface so your original suggestion may be best.

[bobpaw]

I compared the CTest suite resultant meshes in pumi-meshes and build/test/ and they are identical except for build/test/mis_test. However, MIS uses random numbers, so I am figuring they are otherwise identical.

@onkarsahni said:

When srand is used, one should provide option to use a fixed seed (for debugging, testing, regression, etc.) and a different seed (like based on time function), see https://github.com/SCOREC/core/blob/develop/apf/apfMIS.cc#L11.

[jacobmerson]

I mostly focused on the changes in the C++ and CMake code and did not review the C implementation in detail.

I have one major concern that we need to clarify before merging is a more concrete understanding of the semantics of the class's comm ownership. I'm currently against a shared ownership model for this particular case. And, highly against anything that puts the onus of remembering to clean things up on the user. Is there a simple case that can demonstrate why you needed to add the ownscomm logic?

The other more minor concern is that we should get rid of the need for #ifdefs in the user code by providing our own init and finalize routines. That will substantially clean up the executables.

The rest is very minor stuff that should be quick to update.

Otherwise, I think this is very much moving in the right direction. The changes clean up a fair number of places where we have a mix of MPI and PCU use which is bound to cause issues.

[bobpaw]

The other more minor concern is that we should get rid of the need for #ifdefs in the user code by providing our own init and finalize routines. That will substantially clean up the executables.

I was considering adding a specialized PCU::PCU(int *argc, char ***argv) which would do MPI_Init and save a flag to run MPI_Finalize later. That specialization would allow users to avoid introducing a scope just for the PCU object because MPI_Finalize will be guaranteed to be called after any pcu_finalize, etc.

Outside of something like that, what is so bad about having #ifdefs? When we discussed, we thought it would help to make it clear what is going on to people looking at tests as examples.

[cwsmith]

Are the #ifdef SCOREC_NO_MPI conditionals only used in the examples/drivers?

[bobpaw]

Are the #ifdef SCOREC_NO_MPI conditionals only used in the examples/drivers?

@cwsmith, the places with #ifdef SCOREC_NO_MPI are: tests (including "utils"), some exes in phasta/, pcu/pcu_defines.h, and pcu/PCU.h (for pcu::Time and the default constructor).

[jacobmerson]

The thing about ifdefs is it can make the code much more unreadable and the user needs to know under the hood whether they should have MPI or not. If we wrap those ifdefs in our own initialize function then we take care of that subtlety for them.

I was considering adding a specialized PCU::PCU(int *argc, char ***argv) which would do MPI_Init and save a flag to run MPI_Finalize later. That specialization would allow users to avoid introducing a scope just for the PCU object because MPI_Finalize will be guaranteed to be called after any pcu_finalize, etc.

I don't like this solution. The reason we have the PCU object is that we may have many of them lying around with different communicators under the hood, but ultimately still using the same MPI library. I have use cases where this is needed. That constructor means the PCU object may or may not own the MPI initialization. What happens when the owning PCU goes out of scope? All other PCUs that are still alive would break as they would be doing MPI operations after MPI_finalize.

So, there needs to be some library/MPI initialization at a higher level than the PCU object.

[bobpaw]

What happens when the owning PCU goes out of scope? All other PCUs that are still alive would break as they would be doing MPI operations after MPI_finalize.

Shouldn't the world-initializing PCU only go out of scope at the end of the main function? Shouldn't all other objects have been destructed by then?

[bobpaw]

I am not touching apfCGNS code yet because pcgnslib.h has #include "mpi.h" and therefore CGNS is incompatible with SCOREC_NO_MPI. But I will need to add CMake logic related to the incompatibility and update apfCGNS to not use GetMPIComm.

[bobpaw]

We determined that instead of PCU::OwnsComm, we can remove pcu_mpi_t::original_comm entirely and also remove PCU::GetMPIComm and PCU::SwitchMPIComm since those functions only existed because we had to shuffle the MPI_Comm around in the singleton PCU. With the new pcu::PCU C++ object, we do not need such functions.

[bobpaw]

SCOREC_NO_MPI is also likely exclusive with ENABLE_ZOLTAN since zoltan.h has #include <mpi.h>.

[cwsmith]

SCOREC_NO_MPI is also likely exclusive with ENABLE_ZOLTAN since zoltan.h has #include <mpi.h>.

Hmmm. Zoltan can be used locally on each process (see the MPI_COMM = SELF) to do what we called 'local part splitting' but that use case then relies on migrating elements to other processes via ... MPI. So, yes, ENABLE_ZOLTAN requires MPI. Good catch.

[bobpaw]

For when I update apfZoltanCallbacks.cc,

https://github.com/sandialabs/Zoltan/blob/f6361719dd66cac62db8dbed120704e436a5ee81/src/zz/zz_struct.c#L114-L139

Zoltan duplicates the comm, so I should free my reference after Zoltan create.

[bobpaw]

@cwsmith, I'm running into a subtle bug that I'm hoping you can help with. The test case pumi3d-1p is failing intermittently (1/4 or 1/5 of the time) in this new branch. I believe the culprit is a change made in pumi/pumi_mesh.cc, but I cannot figure out how my changes caused this issue. The test is only activated given ENABLE_ZOLTAN, and since this code uses apf::createZoltanSplitter, it is doing the MPI_COMM_SELF splitting like you described. I can additionally share logs of the failing test.

New code:

core/pumi/pumi_mesh.cc

Lines 213 to 241 in 0069474

	pMesh pumi_mesh_load(pGeom g, const char* filename, int num_in_part, const char* mesh_type)
	{
	pcu::PCU *pcu_obj = pumi::instance()->getPCU();
	if (strcmp(mesh_type,"mds"))
	{
	if (!pcu_obj->Self()) std::cout<<"[PUMI ERROR] "<<__func__<<" failed: invalid mesh type "<<mesh_type<<"\n";
	return NULL;
	}
	if (num_in_part==1 && pcu_obj->Peers()>1) // do static partitioning
	{
	int num_target_part = pcu_obj->Peers()/num_in_part;
	bool isMaster = ((pcu_obj->Self() % num_target_part) == 0);
	pMesh m = 0;
	apf::Migration* plan = 0;
	std::unique_ptr<pcu::PCU> split_comm = pcu_obj->Split(
	pcu_obj->Self() % num_target_part, pcu_obj->Self() / num_target_part
	);
	if (isMaster) {
	m = apf::loadMdsMesh(g->getGmi(), filename, split_comm.get());
	plan = getPlan(m, num_target_part);
	}
	if (m != nullptr) m->switchPCU(pcu_obj);
	pumi::instance()->mesh = apf::repeatMdsMesh(m, g->getGmi(), plan, num_target_part, pcu_obj);
	}
	else
	pumi::instance()->mesh = apf::loadMdsMesh(g->getGmi(), filename, pcu_obj);
	pumi_mesh_print(pumi::instance()->mesh);
	return pumi::instance()->mesh;
	}

Old code:

core/pumi/pumi_mesh.cc

Lines 234 to 260 in a900c47

	pMesh pumi_mesh_load(pGeom g, const char* filename, int num_in_part, const char* mesh_type)
	{
	if (strcmp(mesh_type,"mds"))
	{
	if (!pumi::instance()->getPCU()->Self()) std::cout<<"[PUMI ERROR] "<<__func__<<" failed: invalid mesh type "<<mesh_type<<"\n";
	return NULL;
	}
	if (num_in_part==1 && pumi::instance()->getPCU()->Peers()>1) // do static partitioning
	{
	MPI_Comm prevComm = pumi::instance()->getPCU()->GetMPIComm();
	int num_target_part = pumi::instance()->getPCU()->Peers()/num_in_part;
	bool isMaster = ((pumi::instance()->getPCU()->Self() % num_target_part) == 0);
	pMesh m = 0;
	apf::Migration* plan = 0;
	split_comm(num_target_part, *pumi::instance()->getPCU());
	if (isMaster) {
	m = apf::loadMdsMesh(g->getGmi(), filename, pumi::instance()->getPCU());
	plan = getPlan(m, num_target_part);
	}
	merge_comm(prevComm, *pumi::instance()->getPCU());
	pumi::instance()->mesh = apf::repeatMdsMesh(m, g->getGmi(), plan, num_target_part, pumi::instance()->getPCU());
	}
	else
	pumi::instance()->mesh = apf::loadMdsMesh(g->getGmi(), filename, pumi::instance()->getPCU());
	pumi_mesh_print(pumi::instance()->mesh);
	return pumi::instance()->mesh;
	}