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

Cleanup how fields_to_plot = none is handled #3219

Merged
merged 2 commits into from
Jul 9, 2022

Conversation

dpgrote
Copy link
Member

@dpgrote dpgrote commented Jul 7, 2022

There has been a change in AMReX that allows MultiFabs to be created with ncomp = 0. (AMReX PR #2873) This allows the handling the fields_to_plot = none to be cleaned up, removing the if checks around the allocation and call to WriteToFile.

This also fixes another issue that arose in PR #2419, that the plot files were not readable by Yt (because the Header file was not written out). With this PR, that is also fixed.

Once the version of AMReX that WarpX depends on is updated, I remove the WIP and this can be merged.

@dpgrote dpgrote force-pushed the cleanup_fields_to_plot_none branch from 7ae0758 to cc14c52 Compare July 8, 2022 16:24
@@ -540,12 +540,8 @@ FullDiagnostics::InitializeBufferData (int i_buffer, int lev ) {
if (use_warpxba == false) dmap = amrex::DistributionMapping{ba};
// Allocate output MultiFab for diagnostics. The data will be stored at cell-centers.
int ngrow = (m_format == "sensei" || m_format == "ascent") ? 1 : 0;
// The zero is hard-coded since the number of output buffers = 1 for FullDiagnostics
Copy link
Member Author

Choose a reason for hiding this comment

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

Note for reviewer - this comment was left over from previous changes and is no longer meaningful so was delete here for clean up.

@dpgrote dpgrote requested review from ax3l and RemiLehe July 8, 2022 16:33
@EZoni
Copy link
Member

EZoni commented Jul 8, 2022

@dpgrote Is the version of AMReX pulled in #3211 (now merged) good enough to remove the [WIP] tag here?

@EZoni EZoni added bug Something isn't working bug: affects latest release Bug also exists in latest release version component: diagnostics all types of outputs labels Jul 8, 2022
@dpgrote
Copy link
Member Author

dpgrote commented Jul 9, 2022

@EZoni Unfortunately no. The merge was made this morning and is newer than the AMReX version in PR #3211.

@ax3l
Copy link
Member

ax3l commented Jul 9, 2022

Feel free to bump AMReX to the latest version in the current PR via this script:
./Tools/Release/updateAMReX.py

dpgrote and others added 2 commits July 9, 2022 16:32
@ax3l ax3l force-pushed the cleanup_fields_to_plot_none branch from ae7a95d to 864d441 Compare July 9, 2022 14:33
@ax3l ax3l changed the title [WIP]Cleanup how fields_to_plot = none is handled Cleanup how fields_to_plot = none is handled Jul 9, 2022
@ax3l
Copy link
Member

ax3l commented Jul 9, 2022

I went ahead and pushed the AMReX update :)

Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

LGTM, thank you :)

@dpgrote
Copy link
Member Author

dpgrote commented Jul 9, 2022

Thanks @ax3l!

@dpgrote dpgrote merged commit 83a2f49 into ECP-WarpX:development Jul 9, 2022
dpgrote added a commit to RemiLehe/WarpX that referenced this pull request Jul 30, 2022
* For fields_to_plot=none, do allocation with ncomp=0

* Update AMReX

Via
```
./Tools/Release/updateAMReX.py
```

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
@dpgrote dpgrote deleted the cleanup_fields_to_plot_none branch September 2, 2022 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: affects latest release Bug also exists in latest release version bug Something isn't working component: diagnostics all types of outputs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants