Direct mesh access combined with mappings

[davidscn]

Please describe the problem you are trying to solve.
Combining the direct access with mappings has an impact on the defined access region. In our API, we define the following behavior

precice/src/precice/Participant.hpp

Lines 896 to 912 in b9c0728

	* @note If you combine the direct access with a mpping (say you want
	* to read data from a defined mesh, as usual, but you want to directly
	* access and write data on a received mesh without a mapping) you may
	* not need this function at all since the region of interest is already
	* defined through the defined mesh used for data reading. This is the
	* case if you define any mapping involving the directly accessed mesh
	* on the receiving participant. (In parallel, only the cases
	* read-consistent and write-conservative are relevant, as usual).
	*
	* @note The safety factor scaling (see safety-factor in the configuration
	* file) is not applied to the defined access region and a specified safety
	* will be ignored in case there is no additional mapping involved. However,
	* in case a mapping is in addition to the direct access involved, you will
	* receive (and gain access to) vertices inside the defined access region
	* plus vertices inside the safety factor region resulting from the mapping.
	* The default value of the safety factor is 0.5,i.e., the defined access
	* region as computed through the involved provided mesh is by 50% enlarged.

This might lead to confusion when users define an access region and receive vertices beyond the defined access region.

Describe the solution you propose.
Even though we enable users to combine both features, we pre-filter the vertices returned by getVerticesAndIDs according to the user-defined access region. This will modify the current behavior in terms of

• a user must call setMeshAccesRegion regardless of whether there is a mapping or not
• a mapping doesn't affect the vertices we return by this function

Additional context
Relevant for considerations in #2099

How and do we consider this breaking?

Discussed with @uekerman about this.

[davidscn]

I would then remove the notes above and the behavior in terms of

precice/src/precice/Participant.hpp

Lines 896 to 903 in b9c0728

	* @note If you combine the direct access with a mpping (say you want
	* to read data from a defined mesh, as usual, but you want to directly
	* access and write data on a received mesh without a mapping) you may
	* not need this function at all since the region of interest is already
	* defined through the defined mesh used for data reading. This is the
	* case if you define any mapping involving the directly accessed mesh
	* on the receiving participant. (In parallel, only the cases
	* read-consistent and write-conservative are relevant, as usual).

would be: regardless of any mapping, one has to define a bounding box

and for the behavior here

precice/src/precice/Participant.hpp

Lines 905 to 912 in b9c0728

	* @note The safety factor scaling (see safety-factor in the configuration
	* file) is not applied to the defined access region and a specified safety
	* will be ignored in case there is no additional mapping involved. However,
	* in case a mapping is in addition to the direct access involved, you will
	* receive (and gain access to) vertices inside the defined access region
	* plus vertices inside the safety factor region resulting from the mapping.
	* The default value of the safety factor is 0.5,i.e., the defined access
	* region as computed through the involved provided mesh is by 50% enlarged.

of course the region would be fix. Since we filter now actively for the specified region, there is also the usability question what to do about the serial case: as of now, I think calling the function was kept optional

precice/src/precice/Participant.hpp

Lines 887 to 891 in b9c0728

	* @note Defining a bounding box for serial runs of the solver (not
	* to be confused with serial coupling mode) is valid. However, a
	* warning is raised in case vertices are filtered out completely
	* on the receiving side, since the associated data values of the
	* filtered vertices are filled with zero data.

[davidscn]

Also note that the current concept of the direct mesh access is

   // initialize preCICE as usual
    precice.initialize();

    // Get the size of the received mesh partition, which lies within the
    // defined bounding (provided by the coupling participant)
    const int otherMeshSize = precice.getMeshVertexSize(otherMesh);

    // Now finally get information about the mesh vertices.
    // First allocate memory for the IDs and coordinates
    std::vector<double> otherCoordinates(otherMeshSize * dim);
    std::vector<VertexID> otherVertexIDs(otherMeshSize);
    // ... and afterwards ask preCICE to fill the vectors
    precice.getMeshVertexIDsAndCoordinates(otherMesh,
                                           otherVertexIDs,
                                           otherCoordinates);

and with the requested changes, the behavior of getMeshVertexSize will change if we have access enabled.

[davidscn]

@uekerman could you define more clearly the intended behavior of getMeshVertexCoordinatesandIDs.

At the moment, we have

precice/tests/parallel/direct-mesh-access/helpers.cpp

Lines 95 to 103 in 4d1c89f

	// Check, if we can use the 'getMeshVertexIDsAndCoordinates' function on provided meshes as well,
	// though the actual purpose is of course using it on received meshes
	const std::size_t ownMeshSize = interface.getMeshVertexSize(meshName);
	BOOST_TEST(ownMeshSize == size);
	std::vector<int> ownIDs(ownMeshSize);
	std::vector<double> ownCoordinates(ownMeshSize * dim);
	interface.getMeshVertexIDsAndCoordinates(meshName, ownIDs, ownCoordinates);
	BOOST_TEST(ownIDs == ids);
	BOOST_TEST(testing::equals(positions, ownCoordinates));

The function is a pure getter function on whatever we have on our local partition. With the above filtering we would need to add the bounding box filter additionally. However, what is supposed to happen in case we have:

• a serial case and want getMeshVertexCoordinatesandIDs of a received mesh. setMeshAccessRegion required? do we filter? Is the enable-access flag required?
• want getMeshVertexCoordinatesandIDs on a provided mesh in serial and parallel. setMeshAccessRegion required? do we want to filter?
• a parallel case on a received mesh: setMeshAccessRegion even though a mapping is defined, we filter

Allowing getMeshVertexCoordinatesandIDs for provided meshes is a bit of a showstopper right now as the function might behave then rather unexpected in my opinion. I would suggest to

• a) only allow getMeshVertexCoordinatesandIDs and setMeshAccessRegion on received meshes
• b) make the 'enable-access' flag mandatory for calling both functions (we cannot enforce the flag without a) as the option doesn't exist on provided-meshes)
• c) have a serial run as exception, where one can call getMeshVertexCoordinatesandIDs without calling setMeshAccess and we don't apply any filtering (having the getMeshVertexCoordinatesandIDs as a getter function only)
• d) enforce setMeshAccess for parallel runs, even in case a mapping is defined (breaking behavior).

[uekerman]

I agree on a)-d).

How and do we consider this breaking?

My tendency is to consider this non-breaking or non-hurting at least.

We change the functionality in two ways:

• One must call setMeshAccessRegion for a parallel participant if enable-access is set. One could consider this one breaking, but it is definitely not hurting much if we clearly describe in the error message what to do. And it is backwards compatible: always calling setMeshAccessRegion doesn't hurt either.
• One gets fewer values in getMeshVertexCoordinatesandIDs. But this is a usability improvement in my opinion.

[fsimonis]

One gets fewer values in getMeshVertexCoordinatesandIDs. But this is a usability improvement in my opinion.

As @davidscn mentioned above, this means that getMeshVertexSize() needs to return the amount of vertices in the region of interest to use getMeshVertexCoordinatesandIDs().

This function is currently usable for all received and provided meshes. I could imagine users displaying this as debugging information or similar.

As we reused this function from before the direct access feature, it is possible that changing it will either break existing code or lead to surprising results. There is also no mention of this function being tied to direct access.
So, either we go ahead, change the meaning of the function or we add a new one.

[davidscn]

I would go the "safe" path here and make the function throw in case it is called on a received-mesh, unless the flag enabled access to it. In current scenarios, the direct access is the only reason to call it on received meshes in user code. Everything else is probably a bug or misuse.

[uekerman]

I would go the "safe" path here and make the function throw in case it is called on a received-mesh, unless the flag enabled access to it.

Sounds good to me, but make it a warning to avoid breaking erroneous, but working adapter code.

[davidscn]

Sounds good to me, but make it a warning to avoid breaking erroneous, but working adapter code.

What's then the expected return value in this case?

[uekerman]

What's then the expected return value in this case?

What we had so far: the number of vertices of this mesh partition.

[davidscn]

Was implemented in #2099, but waiting until precice/preeco-orga#31 is final before closing here.