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

Ablity to use .pvtu files for parallel postprocessing with ParaView #210

Merged
merged 2 commits into from
Dec 17, 2021

Conversation

mohd-afeef-badri
Copy link
Contributor

No description provided.

pvd << " </Collection>\n";
pvd << "</VTKFile>\n";
}
*name = file_without_extension + "_" + (size > 1 ? str[2].str() + "_" : "") + str[0].str() + (size > 1 ? "_" + /*str[1].str()*/to_string(rank) : "") + "." + extension;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a specific reason why you replaced str[1].str() by to_string(rank)?
I understand this makes the ProcZero definition in plugin/seq/iovtk.cpp easier when mpiSize > 1, but it makes it harder IMHO to browse through files. Is this a ParaView requirement? Otherwise, I'd be happy to revert just this change, and "fix" PvtuWriter and PvdWriter to handle a rank 0 whose tailing string is, e.g., with 16 processes, _00.xyz (instead of _0.xyz).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for reviewing the code.

The reason to use to_string(rank) over str[1].str() was indeed to make it easier to identify the rank 0 in plugin/seq/iovtk.cpp. And also to avoid the reoccurring string assembly via ostringstream within PvtuWriter, since this has to be performed in a for loop of the size mpiSize by a single proc (mpirank=0 which is incharge of writing the pvtu file). There is no ParaView requirement attached to this choice.

If you prefer the str[1].str(), which I agree makes the file browsing easier, we can definitely make changes in PvtuWriter and PvdWriter to make it work. Do you want me to make these changes, and push them (I already tried something locally and they seem to work)?

Copy link
Member

Choose a reason for hiding this comment

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

Sure, feel free to push whatever you want. Be careful though, I force-pushed in your branch to split your initial commit in two. If it's too much trouble for your, you can just send me the patch w.r.t. your local branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made the necessary changes here.

@prj- prj- merged commit b0935d9 into FreeFem:develop Dec 17, 2021
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.

2 participants