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

Add support for writing parallel structured VTK formats (.pvti / .pvtr / .pvts) #98

Merged
merged 18 commits into from
Jan 21, 2022

Conversation

corentin-dev
Copy link
Contributor

Here is a PR to close issue #91

What was added : support for PRectilinearGrid, PStructuredGrid and PImageData.

WholeExtent needs to be improved. I am tempted to suggest using MPI here, but it would add a dependency? Any idea about this?

@corentin-dev
Copy link
Contributor Author

@amartinhuertas maybe you have some ideas on how to improve this PR?

@jipolanco
Copy link
Member

Thanks! I'm not yet sure about the best solution for the WholeExtent issue, but I think it's best to avoid depending on MPI.

I guess one simple solution would be to ask the user to explicitly pass extent and whole_extent as keyword arguments of pvtk_grid when writing structured grids. In parallel jobs, as a user one would like to have control of that anyway. Note that extent is already accepted by vtk_grid, but is not really documented...

@codecov-commenter
Copy link

codecov-commenter commented Jan 17, 2022

Codecov Report

Merging #98 (17fe736) into master (a0344d2) will increase coverage by 0.43%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #98      +/-   ##
==========================================
+ Coverage   95.58%   96.01%   +0.43%     
==========================================
  Files          16       16              
  Lines         770      803      +33     
==========================================
+ Hits          736      771      +35     
+ Misses         34       32       -2     
Impacted Files Coverage Δ
src/gridtypes/common.jl 96.66% <100.00%> (+1.01%) ⬆️
src/gridtypes/pvtk_grid.jl 99.25% <100.00%> (+0.28%) ⬆️
src/gridtypes/structured/imagedata.jl 100.00% <100.00%> (+5.55%) ⬆️
src/gridtypes/structured/rectilinear.jl 100.00% <100.00%> (ø)
src/gridtypes/structured/structured.jl 86.27% <100.00%> (+0.27%) ⬆️
src/gridtypes/array.jl 85.71% <0.00%> (-1.79%) ⬇️
src/gridtypes/unstructured/polyhedron.jl 93.75% <0.00%> (-0.70%) ⬇️
src/gridtypes/unstructured/unstructured.jl 94.59% <0.00%> (-0.15%) ⬇️
src/write_data.jl 96.40% <0.00%> (-0.07%) ⬇️
src/gridtypes/ParaviewCollection.jl 97.72% <0.00%> (-0.06%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0344d2...17fe736. Read the comment docs.

@jipolanco
Copy link
Member

jipolanco commented Jan 21, 2022

I think I got all structured formats working correctly.

For now the partitioning must be specified by passing an extents array, which contains the subdomains associated to each process. In MPI applications, each process must know what is the portion of the domain associated to all other processes. This may require some MPI communications before calling pvtk_grid.

I'm open to other ways of specifying the extents, but I'd prefer if we could avoid depending on MPI. If it's really worth it, we could try to support MPI via Requires.

I'll try to add some examples to the documentation. Besides, one annoying thing about parallel structured grids in VTK is that subdomains must overlap, or otherwise VTK (or at least ParaView) complains when loading the files. I'll mention this in the documentation.

@jipolanco jipolanco changed the title Feature/parallel structured Add support for writing parallel structured VTK formats (.pvti / .pvtr / .pvts) Jan 21, 2022
@corentin-dev
Copy link
Contributor Author

Good, I hope my work helped a bit ;-)

I'll try to use it in a parallel, though I don't have overlap, so we will see how it is handled...

@jipolanco jipolanco merged commit 6ef44e4 into JuliaVTK:master Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants