Implement watch integral

[oguzziya]

This PR implements watch integral functionality which was discussed in #342

• Integral values are calculated based on connectivity information:
  • If face connectivity exists, average of three vertices of the triangle is taken and multiplied with the area of the triangle.
  • If edge connectivity exists, average of two vertices of the edge is taken and multiplied with the length of the edge.
  • If there is no connectivity information, all the values of vertices are summed up.
• For the surface area calculation, getArea() and getLength() member functions are defined for Triangle and Edge classes respectively.
• Unit tests are implemented for:
  • Serial, with all connectivity possibilities
  • Parallel, with all connectivity possibilities
  • Serial, for mesh change (for surface area tracking)
• Integration test for serial setup is implemented.

[uekerman]

Looks good 👍 Thanks for the clean code!

I still have to look at the tests.

One general thing: I thought again a bit about the use cases and I think we have to alter what we want. Let's say we want to compute the total force on a geometry. Then, even with mesh connectivity, we don't want to scale the values.

What I would suggest:

• Let's introduce a boolean config option scale-with-connectivity without a default value.
• If false: we just sum up. If true: we scale. If then no connectivity is given, we sum up again, but print a warning (similar to how we treat NP mapping)

OK?

[oguzziya]

• Changelog is updated.
• Bool config option scale-with-connectivity is added.
• Unit tests are updated accordingly. New unit tests are added to test the new option.
• Integration tests are restructured to test connectivity and scaling option.
• Unused functions are removed.
• Formatting (sorry for too many commits and unrelated files)

Github action says there is a formatting problem but do not say where it is:

Using binary: clang-format
GNU parallel detected.
Checking 374 C/C++ files
Checking 77 XML files
The following files are not formatted correctly:
Error: Process completed with exit code 1.

All right, then. Keep your secrets

[fsimonis]

@oguzziya I fixed the bug on develop.
Sorry about that.

[uekerman]

Great code and great test coverage 👍
The warning still needs fixing, otherwise only minor improvements, see below.
I also tested with Alya in parallel and multiple watch integrals (but no connectivity): the values there make sense.

[uekerman]

Suggested change

	if ((not _isScalingOn) and (_mesh->edges().empty() or _mesh->triangles().empty())) {
	PRECICE_WARN("Watch-integral is configured with scaling option on; however, mesh " << _mesh->getName() << " does not contain connectivity information. Therefore, the integral will be calculated without scaling.");
	}
	if ((_isScalingOn) and (_mesh->edges().empty() or _mesh->triangles().empty())) {
	PRECICE_WARN("Watch-integral is configured with scaling option on; however, mesh " << _mesh->getName() << " does not contain connectivity information. Therefore, the integral will be calculated without scaling.");
	}

Another problem I see is that in 2D the set of triangles is always empty, right?

[oguzziya]

Right. So basically, I think we can directly say if edge container is empty, there is no connectivity.

[uekerman]

yes, that solves the second problem. but the not is also still wrong, right?

[oguzziya]

Oh, I missed it sorry

[uekerman]

@oguzziya Could you also please write some documentation in the new docs. You can add a note "Only available for preCICE version >= v2.2.0" or similar.
Could be integrated here: https://www.precice.org/precice.github.io_future/configuration-watchpoint.html
Or simply a new page similar to this one.
There is no real dependency between this PR and the docs. In particular, let's not block this PR by the docs.

[davidscn]

Tested on the Linux cluster with deal.II - OF:

Attribute "scale-with-connectivity" is required, but was not defined.

I guess a default would be nice. I haven't defined connectivity anyway so that the value should be clear. However, it works also with the option true though I have not specified any connectivity, but nothing changes in the actual data. Without any connectivity, the SurfaceArea entry is also somehow misleading or I don't understand it: how is it computed?

I'm not sure, whether a watchi-integral option makes sense for consistent mappings without connectivity/scale by are. Summing up total displacements or total stress along all vertices is just nonsense. Especially in case of stress data, one might think that actual forces are obtained due to the word integral.
Furthermore, (at least for ALE), the associated frame (material/spatial), should be documented somewhere. In case of OpenFOAM, I could use a stress data set (deal.II) with connectivity from the OF mesh. However, the mapped data from OF is associated to a spatial frame, whereas the 'integration', i.e., scaling is associated to the material frame. But this is probably beyond the scope of this PR.

[oguzziya]

I think the default could be also nice but I am not sure whether we can get connectivity information in the configuration phase. Maybe we raise an error not in the configuration phase but in the watch-integral.

Without any connectivity, the so-called area is simply the summation of all the vertices on the given interface

The points are that you made are pretty important, however, I am not sure how they can be implemented in this framework because it should be somewhat application-independent, then it makes it a little bit user responsibility maybe. However, in either case, more detailed documentation would be useful for specific use cases from the user side, at least as an initial step.

[uekerman]

@davidscn: good points, thanks for the feedback!

I guess a default would be nice. I haven't defined connectivity anyway so that the value should be clear.

The no default option I suggested on purpose to make this difference really explicit to the user. There is no way around that the user should understand what is happening here. Moreover, as @oguzziya pointed out mesh connectivity is not yet available during configuration.

Without any connectivity, the SurfaceArea entry is also somehow misleading or I don't understand it: how is it computed?

That's a good point and I also have second thoughts here. I would suggest to simply remove the column completely if there is no scaling used.

I'm not sure, whether a watchi-integral option makes sense for consistent mappings without connectivity/scale by are. Summing up total displacements or total stress along all vertices is just nonsense. Especially in case of stress data, one might think that actual forces are obtained due to the word integral.

That this true, but not available on data level. preCICE only distinguishes between consistent and conservative mappings. Not whether data is consistent or conservative. But these are important points for the documentation.

Furthermore, (at least for ALE), the associated frame (material/spatial), should be documented somewhere. In case of OpenFOAM, I could use a stress data set (deal.II) with connectivity from the OF mesh. However, the mapped data from OF is associated to a spatial frame, whereas the 'integration', i.e., scaling is associated to the material frame. But this is probably beyond the scope of this PR.

Good point again. Should also go into the documentation.

[oguzziya]

Currently, SurfaceArea output is removed from the output file for meshes with no connectivity. Since connectivity information is not available during configuration, the initialize() function is added to add SurfaceArea information to the output.

[uekerman]

All looks good now 👍

[precice-bot]

This pull request has been mentioned on preCICE Forum on Discourse. There might be relevant details there:

https://precice.discourse.group/t/highlights-of-the-new-precice-release-v2-2/429/1