Add POpenFOAMReader

[Failxxx]

Overview

We would like to be able to import and manipulate OpenFOAM files which are produced from a parallel computation. Thus, we need to expose the vtkPOpenFOAMReader (class definition).

It has been discussed here: #924

This PR resolves #2590

Details

A first idea was to replace the reader in the current OpenFOAMReader because vtkPOpenFOAMReader inherits from vtkOpenFOAMReader in vtk. Thus we would just need to add the CaseType property to allow users to chose whether reconstructed mesh or decomposed mesh should be read.

However since we are not 100% sure vtkPOpenFOAMReader can propose the exact same services as the vtkOpenFOAMReader we decided to create a class (POpenFOAMReader) which inherits from OpenFOAMReader with a new property (case_type).

Work to be done

• Add POpenFOAMReader to the list of available readers
• Write basic tests
• Find a testing dataset to test more advanced properties when using the case_type property

[codecov]

Codecov Report

Merging #2676 (fe58d39) into main (703278b) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2676      +/-   ##
==========================================
+ Coverage   93.76%   93.78%   +0.02%     
==========================================
  Files          75       76       +1     
  Lines       16148    16213      +65     
==========================================
+ Hits        15141    15206      +65     
  Misses       1007     1007              

[MatthewFlamm]

When I test this out using case_type set to 0 on the dataset in the vtk issue, I get seg faults since I'm using vtk==9.1.0. Do we have to wait for the next vtk release with your upstream fix for this?

Setting case_type to 1 leads to no seg fault, but no data arrays are loaded.

[Failxxx]

I added a small workaround following instructions given in the vtk issue. Now it seems to be working for me. We can see the separation on the mesh.

With case_type = 0 (decomposed mesh)

With case_type = 1 (reconstructed mesh)

[MatthewFlamm]

This is great work!

Add POpenFOAMReader to the list of available readers (I think you know how @MatthewFlamm)

You need to add to pyvista.utilities.__init__ and to doc/api/readers/index.rst.

[MatthewFlamm]

I was also able to switch the existing tests that use a non parallel data set to use POpenFOAMReader with success, so I'm okay if we replace the default reader in CLASS_READER with POpenFOAMReader.

For testing decomposed datasets in a non adhoc way, we'd need to find a dataset that has a permissive license.

[Failxxx]

For testing I have the following file: windAroundBuildings.zip
It is free to use (I added a LICENSE.txt file in it) and comes from here
Should I open a PR in https://github.com/pyvista/vtk-data to add it to the OpenFOAM.zip file?

[MatthewFlamm]

For testing I have the following file: windAroundBuildings.zip
It is free to use (I added a LICENSE.txt file in it) and comes from here
Should I open a PR in https://github.com/pyvista/vtk-data to add it to the OpenFOAM.zip file?

The tutorial data directly from OpenFOAM might be considered licensed the same, i.e. GPL licensed. This would be a problem if so.

[Failxxx]

For testing I have the following file: windAroundBuildings.zip
It is free to use (I added a LICENSE.txt file in it) and comes from here
Should I open a PR in https://github.com/pyvista/vtk-data to add it to the OpenFOAM.zip file?

The tutorial data directly from OpenFOAM might be considered licensed the same, i.e. GPL licensed. This would be a problem if so.

Ok I understand. You are right, this example is GPL licensed so it won't work. I'm trying to find another file.

[MatthewFlamm]

If we cannot find a suitable testing dataset, then we could at least test that the case_type can be set to both values and that an error is raise for an unsupported value.

[MatthewFlamm]

This is looking good to me. I won't be able to do an ad hoc test, i.e an outside the testing suite test of this for at least a week.

We should push users to use this class in the documentation as it is more general. Here is one such place:

pyvista/examples/99-advanced/openfoam-example.py

Line 22 in 975645d

reader = pyvista.OpenFOAMReader(filename)

[MatthewFlamm]

I have tested this ad hoc on a closed source decomposed dataset and it works great. It also passes all tests with the reconstructed or serial generated dataset that is available. I only have one last optional suggestion.

[akaszynski]

I see 2/3 tasks complete, but approved. I'm starting to pare down the number of active PRs in preparation for a minor release sometime this month. There's a scipy tutorial next month that we're working on preparing for; will need a release likely for that.

[akaszynski]

@Failxxx, in your screenshot in this comment, what are you using for the viewer? Is that VTK?

[MatthewFlamm]

I see 2/3 tasks complete, but approved.

The last task was to add a decomposed dataset for testing. I do not have an open source one to add. I tested on a closed source dataset. I'm not sure if @Failxxx is still planning on adding one.

[Failxxx]

@Failxxx, in your screenshot in this comment, what are you using for the viewer? Is that VTK?

No, it's Blender.

[Failxxx]

I see 2/3 tasks complete, but approved.

The last task was to add a decomposed dataset for testing. I do not have an open source one to add. I tested on a closed source dataset. I'm not sure if @Failxxx is still planning on adding one.

I am planning to add one but I can't for the moment. Do you think we can finish this PR before I add it? I am expecting to have one in the 2 next weeks but not 100% sure.

[akaszynski]

I am planning to add one but I can't for the moment. Do you think we can finish this PR before I add it? I am expecting to have one in the 2 next weeks but not 100% sure.

I'd prefer merging this PR and creating a second one with the additional dataset. If it's a large dataset, let's create an example. If small, just a unit test.

For those of you who are wondering why I'm hell bent on getting PRs merged in short time, it's all about Short Lived Feature Branches in keeping with Trunk Based Development. There it's stated:

One key rule is the length of life of the branch before it gets merged and deleted. Simply put, the branch should only last a couple of days. Any longer than two days, and there is a risk of the branch becoming a long-lived feature branch (the antithesis of trunk-based development).

---

Since we're an open source, distributed community with no genuine deadlines other than the one we impose upon ourselves, I'm fine with lengthening the lifetime of active PRs to as long as they need to be fully reviewed. That being said, I'd still prefer short lived feature or bug fix branches and PRs. If additional features need to be added or additional refactoring needs to be done, it's best to split into multiple PRs.

[akaszynski]

This is good to go as-is.

[MatthewFlamm]

Given multiple approvals over multiple days, I'm going to merge this now. Thanks @Failxxx!