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

Fix ParticleHistogram2D #4779

Merged
merged 2 commits into from
Mar 25, 2024
Merged

Conversation

pordyna
Copy link
Contributor

@pordyna pordyna commented Mar 15, 2024

Without m_h_data_2D.clear(); the host memory usage continues growing over the simulation until the process gets killed.
(Observed in 24.3 and 24.2)

i.close(); and series.close(); were my first suspicion. It didn't help, but it is still probably a good idea to explicitly close the iteration and series.

@ax3l ax3l added bug Something isn't working bug: affects latest release Bug also exists in latest release version component: diagnostics all types of outputs labels Mar 15, 2024
@ax3l ax3l requested review from ax3l and WeiqunZhang March 15, 2024 22:24
@@ -138,6 +138,7 @@ void ParticleHistogram2D::ComputeDiags (int step)
Array<int,2> tlo{0,0}; // lower bounds
Array<int,2> thi{m_bin_num_abs-1, m_bin_num_ord-1}; // inclusive upper bounds
amrex::TableData<amrex::Real,2> d_data_2D(tlo, thi);
m_h_data_2D.clear();
Copy link
Member

@ax3l ax3l Mar 15, 2024

Choose a reason for hiding this comment

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

@WeiqunZhang do you see why we leaked (host) memory here?

Copy link
Member

Choose a reason for hiding this comment

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

It's an amrex bug. 😊 AMReX-Codes/amrex#3807

Copy link
Member

@ax3l ax3l Mar 25, 2024

Choose a reason for hiding this comment

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

WarpX PR #4800 should have brought in the fix from AMReX now.

@WeiqunZhang
Copy link
Member

For the initialization of h_table_data, could we switch the order of the two for loops? Because TableData has column major ordering, we want to make the i-loop the innermost loop.

Co-authored-by: Weiqun Zhang <WeiqunZhang@lbl.gov>
WeiqunZhang added a commit to AMReX-Codes/amrex that referenced this pull request Mar 21, 2024
@ax3l
Copy link
Member

ax3l commented Mar 25, 2024

#4800 should have brought in the fix for AMReX-Codes/amrex#3807

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.

Thank you! :)

@ax3l ax3l merged commit d7b1aba into ECP-WarpX:development Mar 25, 2024
45 checks passed
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