Spatially variable Robin BC bug fixes

[aabrown100-git]

Problem

In a recent pull request #437, I added capability to define a spatially variable Robin boundary condition. After attempting to use it with a more complex simulation, I encountered some bugs/inconsistencies

1. To provide the vtp file containing the spatially varying values, the user defines a boundary condition xml element <Robin_vtp_file_path>. This is inconsistent with other boundary conditions that define an xml element <Spatial_values_file_path>.

2. If mesh_scale_factor is not 1, the point matching between to the vtp file and the face object associated with the boundary condition fails

Solution

1. Get rid of <Robin_vtp_file_path> and use <Spatial_values_file_path> instead to define a spatially variable boundary condition.

2. Pass mesh_scale_factor into the BoundaryCondition class and scale coordinates so that point matching is successful.

Additional context

No response

Code of Conduct

• I agree to follow this project's Code of Conduct and Contributing Guidelines

[ktbolt]

@aabrown100-git Note that Spatial_values_file_path files could (should ?) have GlobalNodeID and GlobalElementID VTK DataArrays.

[ktbolt]

@aabrown100-git Using VTK format files for boundary conditions just complicates things, really just need node IDs with values. This is the problem with using a file format intended for one purpose (visualization) for another sort of unrelated purpose (finite element boundary conditions).

It would be good to remove using VTK format files at some point.

[aabrown100-git]

@ktbolt I think using VTK format files for boundary conditions makes it easier for the user.

Setting node IDs feels to me more prone to errors. For example, I'm not sure whether to index from 0 or from 1. Also, the user would need to write extra code to visually verify that the node ID to value mapping produces the desired spatial profile for a boundary condition.

Of course, these could be mitigated with good documentation and some provided scripts, but I just feel using a VTK formal file is a cleaner solution.

With VTK format files that match nodal positions, what you see in Paraview is what you are providing to svMultiPhysics. It complicates things a bit for the developer (mesh_scale_factor, parallelization), but I don't expect to encounter many more issues with it. Also, performing the nodal position matching is only done once at the beginning of the simulation and apparently doesn't take that long.

[mrp089]

I agree with @aabrown100-git. For me, the ideal solution would even go further and have only one vtu file as input that contains all the information we currently outsource to vtps, like surface definitions. You only would need to generate one file that you can easily check in ParaView.

[ktbolt]

@mrp089 @aabrown100-git You are used to using VTK so it seems like a natural choice. However, most users I have interacted with in the SV Forum don't know anything about VTK and they don't know how to program, even write Python scripts.

When I first started working on SimVascular I was surprised that VTK-format files were used to define the mesh. Note that svSolver does not directly use VTK files but had a preprocessing stage to convert the data to a binary format. All solvers I have experience with used node/element ID lists for boundary conditions.

So the future plan will be to remove VTK from svMultiPhysics and use a text/binary file for input. We would support writing results in VTK-format using a plugin. We could still support using VTK for mesh files again using a plugin. I will open an Issue for this related to #219.

@mrp089 Having a single VTU file for input is not a bad idea and probably easier to generate. It would waste space but that is not a big issue I would guess.