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

[WIP] Fix: RedistributeCPU for SoA #3291

Closed
wants to merge 1 commit into from

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented May 2, 2023

Summary

This is a first draft and needs to be generalized.

Additional background

Follow-up to #2878.
ImpactX w/ OMP now: ECP-WarpX/impactx#348

  • not crashing with space charge anymore
  • results still off expectations: we loose particles now.

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

@ax3l ax3l added the bug label May 2, 2023
@ax3l ax3l requested a review from atmyers May 2, 2023 21:16
@ax3l ax3l force-pushed the fix-redistribute-cpu-soa branch from 0191b5e to 7f3f950 Compare May 2, 2023 21:30
@@ -1676,7 +1676,7 @@ ParticleContainer_impl<ParticleType, NArrayReal, NArrayInt, Allocator>
// We own it but must shift it to another place.
auto index = std::make_pair(pld.m_grid, pld.m_tile);
AMREX_ASSERT(tmp_local[pld.m_lev][index].size() == num_threads);
tmp_local[pld.m_lev][index][thread_num].push_back(p);
//tmp_local[pld.m_lev][index][thread_num].push_back(p);
Copy link
Member Author

@ax3l ax3l May 2, 2023

Choose a reason for hiding this comment

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

What is this for a pure SoA particle?
Does the later

aos.insert(aos.end(), aos_tmp[i].begin(), aos_tmp[i].end());
aos_tmp[i].erase(aos_tmp[i].begin(), aos_tmp[i].end());

work as we expect?

Let's add more inline comments for the individual code blocks, so we understand the in-transit data structure and logic each of the many loops/copies/moves executes :)

Not clear to me if in-transit it is still an SoA and when what is overwritten. I see that later AoS attributes are not communicated correctly in ImpactX (e.g., momenta) but cannot say why yet.

@ax3l
Copy link
Member Author

ax3l commented May 4, 2023

Underlying bug fixed via #3296.

We keep this open since we want to check if sections like

tmp_local[pld.m_lev][index][thread_num].push_back(p);

are well-defined/sensible for pure SoA particles or if we need additional branching there.

@ax3l
Copy link
Member Author

ax3l commented May 9, 2023

Continued in #3299

@ax3l ax3l closed this May 9, 2023
@ax3l ax3l deleted the fix-redistribute-cpu-soa branch May 9, 2023 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants