search

FluidityProject / fluidity

Fluidity
http://fluidity-project.org
Other
367 stars 115 forks source link

ENH: Update VTK Ghost API #354

Closed Patol75 closed 2 years ago

Patol75 commented 2 years ago

This PR is a port of changes originally written for this commit; hopefully, I did not break too many things in the process.

The purpose of this PR is to reflect already old changes in the VTK API regarding ghosts. Currently, loading a Fluidity PVTU on recent versions of ParaView or using VTK bindings for Python will generate as many error messages as CPU cores were used, which is not a good sign. Furthermore, pvtu2vtu is broken when attempting to generate multiple VTUs at once.

The changes included in this PR should correct these issues. The idea is for the CI to stay green and for the reviewer(s) to generate a parallel model using this branch and assert that results can be properly loaded into ParaView and that pvtu2vtu works.

To review the changes, please have a look at the second commit attached; the first one only deals with formatting, no changes were applied.

Patol75 commented 2 years ago

@angus-g This one is actually a massive pain to rebase, mostly because of the Python formatting changes. Any chance to proceed as-is?

Patol75 commented 2 years ago

Thanks, @angus-g.

I have just pushed a pre-commit branch. For now, all I have done is patch files using information detailed here. CI should follow based on this. Actions run will be here.

angus-g commented 2 years ago

Hmm, I rebased this onto main just to get the PR in, but it seems like the new test failure is at least somewhat real?

Patol75 commented 2 years ago

This test passed in the previous Actions run. Additionally, I remember seeing similar errors when I went through all the tests for the CMake build. Unfortunately, I do not remember what I did in that case if anything. I cannot see any major changes for that particular test in the CMake branch, and I do not think that the changes in the present branch affect this test at all. I am thinking it could well turn green if triggered again.

Patol75 commented 2 years ago

Thanks @angus-g, all green :)