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 wrapper for VisMF #388

Merged
merged 5 commits into from
Nov 1, 2024
Merged

Conversation

dpgrote
Copy link
Contributor

@dpgrote dpgrote commented Nov 1, 2024

I need to read in a MultiFab so added this.

@dpgrote dpgrote added the enhancement New feature or request label Nov 1, 2024
@WeiqunZhang
Copy link
Member

There is a potential issue. The VisMF::Read wrapper always returns a freshly made MultiFab. The problem of that is if a users calls the wrapper twice to read two MulitiFabs, there is no guarantee that the two will have the same DistributionMapping. That may not be what the user expects. So it might be better to also add the option of reading the data into a preexisting MultiFab.

@dpgrote
Copy link
Contributor Author

dpgrote commented Nov 1, 2024

If the two MultiFabs files are from MultiFabs with the same box array layout and distribution map, shouldn't they have the same distribution map when they are read back in (or at least the two should have the same as each other)?

I added a second version of Read that takes a MultiFab as an argument.

@WeiqunZhang
Copy link
Member

The DistributionMapping is not saved in the VisMF files. For flexibility in load balancing, using the same boxarray to build distributionmapping does not always produce the same result.

However, we can probably make an exception in VisMF::Read to make it less error prone for the users.

@WeiqunZhang WeiqunZhang merged commit 6c2dc4f into AMReX-Codes:development Nov 1, 2024
18 checks passed
@dpgrote dpgrote deleted the add-VisMF branch November 1, 2024 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants