Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for VTK #15266

Merged
merged 2 commits into from Jun 8, 2023
Merged

Added support for VTK #15266

merged 2 commits into from Jun 8, 2023

Conversation

pcafrica
Copy link
Contributor

@pcafrica pcafrica commented May 24, 2023

After a discussion with @luca-heltai and @fdrmrc, we would like to add support for the VTK library in deal.II.

VTK can be used for importing scalar or vector fields, evaluate/interpolate/project them onto a triangulation and many other geometry handling tasks. We plan to support versions higher or equal to 9.0.0. These features should complement the use of CGAL.

VTK has already been extensively tested and used within lifex and we aim to port existing functionalities from lifex to deal.II. I expect many more PR to follow once this is accepted.

TODO:

  • check workflow file: is it ok to update to Ubuntu 22.04?
  • add sample class
  • add tests

@pcafrica pcafrica marked this pull request as draft May 24, 2023 20:51
@pcafrica pcafrica changed the title [WIP] Added support for VTK Added support for VTK May 24, 2023
@bangerth
Copy link
Member

I'll let @tamiko do the review.

Out of curiosity, what features in VTK do you plan on using?

@pcafrica
Copy link
Contributor Author

I'll let @tamiko do the review.

Out of curiosity, what features in VTK do you plan on using?

Most importantly, reading a (surface or volume) field from a VTK file and interpolating/projecting it at triangulation nodes using closest-point distance, linear interpolation or signed distance. This can be used for, e.g., imposing boundary conditions, reading a mesh displacement from file, defining different material properties or defining an immersed surface.

This will involve VTK filters like cell and point locators. An example can be found here, where, inheriting from Function<dim>, we are able to evaluate an input field at an arbitrary triangulation point.

@bangerth
Copy link
Member

Ah, very cool -- something like FEFieldFunction, just with externally provided data! That is something that I'm sure we can also put to good use in ASPECT, for example. I'm looking forward to the patch!

@bangerth
Copy link
Member

/rebuild

@agrayver
Copy link
Contributor

agrayver commented May 25, 2023

Is there also a possibility here to export/import refined triangulations in a backward-compatible way? Current approach based on the boost-serialization library is not backward compatible and, in practice, breaks with each major release (say meshes created/saved with 9.3 cannot be read with 9.4 and so on...).

@pcafrica
Copy link
Contributor Author

Is there also a possibility here to export/import refined triangulations in a backward-compatible way? Current approach based on the boost-serialization library is not backward compatible and, in practice, breaks with each major release (say meshes created/saved with 9.3 cannot be read with 9.4 and so on...).

Personally I've never used VTK for this purpose, but it's something worth investigating for sure.

@pcafrica pcafrica force-pushed the vtk_support branch 5 times, most recently from 4ca056c to 7cfb2e0 Compare May 25, 2023 11:41
@pcafrica pcafrica marked this pull request as ready for review May 25, 2023 14:43
@pcafrica
Copy link
Contributor Author

pcafrica commented May 25, 2023

A note about the Linux workflow: installing libvtk9-dev from apt results in a dependency conflict that I wasn't able to solve: https://github.com/dealii/dealii/actions/runs/5073291921/jobs/9116267080?pr=15266#step:3:86

In particular, libvtk9-dev depends on libtbb-dev, therefore it is incompatible with libtrilinos-all-dev which depends on libtbb2-dev

The current workaround is to build VTK manually, but this takes ~45 minutes and I guess it is too much (?).

@luca-heltai
Copy link
Member

Can you try to add at the end

apt-get install ...

libvtk9-dev \
libtbb-dev-

(notice the - at the end of name of the missing package). I think we do install libtbb somewhere, since dealii depends on it. Maybe the naming is not right in libvtk9-dev.

@pcafrica
Copy link
Contributor Author

Can you try to add at the end

apt-get install ...

libvtk9-dev \
libtbb-dev-

(notice the - at the end of name of the missing package). I think we do install libtbb somewhere, since dealii depends on it. Maybe the naming is not right in libvtk9-dev.

Thanks for the hint! Unfortunately, libvtk9-dev cannot be installed without libtbb-dev.

PS: libtbb2-dev is installed as a Trilinos dependency.

@pcafrica pcafrica force-pushed the vtk_support branch 3 times, most recently from fb0270f to a39d21f Compare May 25, 2023 19:13
@pcafrica
Copy link
Contributor Author

Another solution I've come up with is to define a new workflow, which only builds the VTK wrappers. Opinions?

@luca-heltai
Copy link
Member

My preference would be to add vtk as a dependency in the docker image dealii/dependencies:jammy (or focal) and use that one to build one of the workflow.

@pcafrica
Copy link
Contributor Author

Let's wait for #15278 to be discussed/merged before continuing with this PR.

@luca-heltai
Copy link
Member

Thanks Pasquale!

@luca-heltai
Copy link
Member

Would you mind rebasing and squashing into a couple of significative commits?

@pcafrica pcafrica force-pushed the vtk_support branch 4 times, most recently from bce0e58 to a446dd0 Compare May 28, 2023 20:47
@pcafrica
Copy link
Contributor Author

Would you mind rebasing and squashing into a couple of significative commits?

Done! From my side, it looks ready to merge.

@luca-heltai
Copy link
Member

@tamiko, would you mind taking a look at the cmake stuff? I am fine with everything, but you are working on restructuring stuff....

@luca-heltai
Copy link
Member

Also, @pcafrica, would you mind adding VTK support also in contrib/docker/Dockerfile? You did this already in the docker-files repository, but this one is used by the master workflow to create docker images of master after each merge to master.

@pcafrica
Copy link
Contributor Author

pcafrica commented May 29, 2023

Done!

PS: the pipeline fails with error mkdir: cannot create directory ‘build’: Permission denied. I think this is due to GitHub setting HOME=/github/home, which in the Docker image belongs to a different user.

We could try adding

    defaults:
      run:
        working-directory: /home/dealii

but I don't know if this has implications on the GitHub side.

@luca-heltai any clues?

@luca-heltai
Copy link
Member

luca-heltai commented May 29, 2023

Ah, yes. You have to run the pipeline as user root:

In the workflow:

container:
      image: dealii/dependencies:jammy
      options: --user root

This should work.

@luca-heltai
Copy link
Member

I think we also need to set

container:
      image: dealii/dependencies:jammy
      options: -u root -e OMPI_ALLOW_RUN_AS_ROOT=1 -e OMPI_ALLOW_RUN_AS_ROOT_CONFIRM=1

@masterleinad
Copy link
Member

Please fix the merge conflicts.

foreach(_configuration ${_configurations})
get_target_property(_imported_location ${_library} IMPORTED_LOCATION_${_configuration})
list(APPEND _libraries ${_imported_location})
endforeach()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let us merge this for now
We should support simply listing all targets now, though - but I suggest we try this out in a follow-up pull request.

@luca-heltai luca-heltai merged commit 9d9ae9a into dealii:master Jun 8, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants