0 indexed meshes(#114)

[zasexton]

Arbitrary Mesh Indexing for Problem Initialization

Current situation & Problem

In response to issue #114. Face mesh data for svFSIplus assumes that GlobalNodeID and GlobalElementID are indexed at 1. This scheme is inconsistent with how VTK tools external to SimVascular construct connectivity data on meshes. The different scheme thus makes it difficult to build models in a scripted fashion either though SimVascular's Python VTK functions or external VTK functions.

Release Notes

• arbitrary element type to be correctly initialized by the solver
• agreement with internal and external mesh generation routines

Documentation

No new functions are added through this pull. Instead changes are introduced to existing functions for importing and handling mesh initialization.

Testing

Further testing will be needed for different element types beyond tetrahedral and quadrilateral. In addition, changes introduced to the code should be checked for clarity and minimal code needed for fast implementation. I've included my thoughts as I wrote these changes in #114.

Code of Conduct & Contributing Guidelines

• I agree to follow the Code of Conduct and Contributing Guidelines.

[zasexton]

I don't think I can assign reviewers but it would be nice to have @ktbolt and @mrp089 look over this pull request since I've talked to each of you about this issue.

[ktbolt]

@zasexton You can't merge code that is not fully tested and verified to work for all supported element types.

[zasexton]

@ktbolt yes, I agree. currently the code has been tested on elements from the current test cases supplied by svFSIplus. I am still reviewing initialization for problems involving fiber mesh files. For elements including tets, quads, hex, and triangles the current code is currently undergoing the integration tests. For other elements such as wedges and shells, I am not sure that we have a test that uses those elements yet? If so then the code will be tested against them but if not I wanted to be clear that there may be some types of elements that may lack the appropriate tests.

[mrp089]

@zasexton, congrats on your first pull request, and thank you for nicely documenting what you have changed!

The integration tests timed out at ~6 h (they usually take ~5 min). You can run them locally with (you don't need to create a conda environment):

svFSIplus/.github/workflows/test.yml

Lines 40 to 41 in 9723a97

	cd tests
	conda run -n svfsiplus pytest -v --durations=0

That will give you an idea of which ones take particularly long.

In terms of testing element types, you can have a look at the most recent successful pipeline and download its coverage report. You can also generate the coverage report locally by running

• svFSIplus/.github/workflows/test.yml

  Lines 30 to 32 in 9723a97

  	cd build
  	cmake -DENABLE_COVERAGE=ON ..
  	make -j2

• Execute any svFSI+ example you'd like to see the coverage on

• svFSIplus/.github/workflows/test.yml

  Lines 45 to 46 in 9723a97

  	cd build/svFSI-build
  	make coverage

A good indication of what elements are currently tested is looking at which lines we hit in nn_elem_gnn.h. You can see that many are still missing, which is not your fault! Unfortunately, we don't know if they are working in their current state either, and we need to add tests.

Some elements might be included when migrating more tests from svFSI-Examples (#95). @ktbolt, can you provide any tests you ran when implementing the elements? Then, we could add those to the testing framework as well.

[zasexton]

When testing cases locally, all but the follow test cases succeed:

• pipe3D_RCR
• dye_AD

All of the above tests fail with the following output for both the 0-indexed-meshes(#114) branch as well as the main avFSIplus branch:

terminate called after throwing an instance of 'std::runtime_error'
  what():  Error reading values for the temporal values file 'lumen_inlet.flow'.for line '

[ktbolt]

@zasexton It seems that read_temporal_values fails because it is reading a blank line.

[mrp089]

That is weird, given the current main branch passes on all test machines. What OS are you using, @zasexton?

[zasexton]

yes, this is a bit confusing for me since lumen_inlet.flow does not contain a blank line when I inspect it. But, I can look more at this.

[zasexton]

I've been running on Windows WSL Ubuntu 20.04.1 LTS

[zasexton]

After testing on a different linux machine instead of Windows WSL, changes in this pull request cause issues in the cmm_3d_pipe and niederer_benchmark_ECGs_quadrature test cases. The niederer_benchmark_ECGs_quadrature benchmark specifically hangs for a long period of time. I believe this correctly reproduced the issues that were seen on the Github actions. I will close this pull request as there is still work to do to satisfy these tests.