Skip empty particle tiles in operator++ to avoid race condition in constructor. - Githubissues

AMReX-Codes / amrex

AMReX: Software Framework for Block Structured AMR

https://amrex-codes.github.io/amrex

Other

536 stars 343 forks source link

Skip empty particle tiles in operator++ to avoid race condition in constructor. #3951

Closed atmyers closed 4 months ago

atmyers commented 4 months ago

The race condition was found by @lucafedeli88 using an automated tool.

Summary

Additional background

Checklist

The proposed changes:

[x] 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

lucafedeli88 commented 4 months ago

@atmyers , thanks a lot for working on this! For the moment I've left just one comment. I am currently trying to test your PR with the thread sanitizer to see if it fixes all the issues.

WeiqunZhang commented 4 months ago

I think we can simplify ParIter by getting rid of m_pariter_index, m_valid_index and m_particle_tiles and adding ParticleTilePtr m_particle_current_tile = nullptr;. As for opeator++, we don't need separate versions from OMP. It can be something like

    void operator++ ()
    {
        m_particle_current_tile = nullptr;
        auto const& particles = m_pc.GetParticles(level);
        do
        {
            MFIter::operator++();
            auto key = std::make_pair(index(), LocalTileIndex());
            auto f = particles.find(key);
            if (f != particles.end() && f->second.numParticles() > 0) {
                m_particle_current_tile = &(f->second);
            }
        }
        while (isValid() && m_particle_current_tile == nullptr);
    }

(Code not tested.)

WeiqunZhang commented 4 months ago

The code snippet I posted has a bug. Cannot call index() if it's not valid. So maybe it's easier to just use while (true) loop and break if it's either invalid or a non-empty tile is found.

lucafedeli88 commented 4 months ago

The thread sanitizer test seems to still see an issue: https://github.com/ECP-WarpX/WarpX/actions/runs/9210693739/job/25338291461 I compiled WarpX using this branch of AMReX: AMReX repository: https://github.com/atmyers/amrex.git (0cf6d9bf4aebc2bf2e9547ec8afc28733d24def4) and I still get something like:

WARNING: ThreadSanitizer: data race (pid=14679)
  Read of size 8 at 0x7b0c0001d428 by thread T3:
    #0 std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::_Select1st<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::size() const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_tree.h:1034:24 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x562283) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #1 std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::_Select1st<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::_M_get_insert_hint_unique_pos(std::_Rb_tree_const_iterator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::pair<int, int> const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_tree.h:2220:8 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x562283)
    #2 std::_Rb_tree_iterator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>> std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::_Select1st<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::_M_emplace_hint_unique<std::piecewise_construct_t const&, std::tuple<std::pair<int, int>&&>, std::tuple<>>(std::_Rb_tree_const_iterator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::piecewise_construct_t const&, std::tuple<std::pair<int, int>&&>&&, std::tuple<>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_tree.h:2463:15 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x56277a) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #3 std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::operator[](std::pair<int, int>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_map.h:532:15 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c24c) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #4 amrex::ParticleContainer_impl<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator, amrex::DefaultAssignor>::DefineAndReturnParticleTile(int, int, int) /__w/WarpX/WarpX/build/_deps/fetchedamrex-src/Src/Particle/AMReX_ParticleContainer.H:1147:9 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c24c)
    #5 ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries(MultiParticleContainer&) (.omp_outlined_debug__) /__w/WarpX/WarpX/Source/Particles/ParticleBoundaryBuffer.cpp:399:61 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5540d2) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #6 ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries(MultiParticleContainer&) (.omp_outlined) /__w/WarpX/WarpX/Source/Particles/ParticleBoundaryBuffer.cpp:392:1 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5540d2)
    #7 __kmp_invoke_microtask <null> (libomp.so.5+0xdbb82) (BuildId: bc9f8970ab3c208335f64564550b062c0d9eb665)
    #8 ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries(MultiParticleContainer&) /__w/WarpX/WarpX/Source/Particles/ParticleBoundaryBuffer.cpp:392:26 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x553d75) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)

  Previous write of size 8 at 0x7b0c0001d428 by main thread:
    #0 std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::_Select1st<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::_M_insert_node(std::_Rb_tree_node_base*, std::_Rb_tree_node_base*, std::_Rb_tree_node<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>*) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_tree.h:2387:7 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x562822) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #1 std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::_Select1st<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::_Auto_node::_M_insert(std::pair<std::_Rb_tree_node_base*, std::_Rb_tree_node_base*>) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_tree.h:1657:21 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x562822)
    #2 std::_Rb_tree_iterator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>> std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::_Select1st<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::_M_emplace_hint_unique<std::piecewise_construct_t const&, std::tu==================
WARNING: ThreadSanitizer: data race (pid=14678)
  Read of size 4 at 0x7b5400007fa0 by thread T3:
    #0 bool std::operator<<int, int>(std::pair<int, int> const&, std::pair<int, int> const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_pair.h:836:18 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c1d0) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #1 std::less<std::pair<int, int>>::operator()(std::pair<int, int> const&, std::pair<int, int> const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_function.h:408:20 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c1d0)
    #2 std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::_Select1st<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::_M_lower_bound(std::_Rb_tree_node<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>*, std::_Rb_tree_node_base*, std::pair<int, int> const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_tree.h:1952:7 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c1d0)
    #3 std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::_Select1st<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::lower_bound(std::pair<int, int> const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_tree.h:1271:16 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c1d0)
    #4 std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::lower_bound(std::pair<int, int> const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_map.h:1309:21 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c1d0)
    #5 std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::operator[](std::pair<int, int>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_map.h:529:17 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c1d0)
    #6 amrex::ParticleContainer_impl<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator, amrex::DefaultAssignor>::DefineAndReturnParticleTile(int, int, int) /__w/WarpX/WarpX/build/_deps/fetchedamrex-src/Src/Particle/AMReX_ParticleContainer.H:1147:9 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c1d0)
    #7 ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries(MultiParticleContainer&) (.omp_outlined_debug__) /__w/WarpX/WarpX/Source/Particles/ParticleBoundaryBuffer.cpp:399:61 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5540d2) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #8 ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries(MultiParticleContainer&) (.omp_outlined) /__w/WarpX/WarpX/Source/Particles/ParticleBoundaryBuffer.cpp:392:1 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5540d2)
    #9 __kmp_invoke_microtask <null> (libomp.so.5+0xdbb82) (BuildId: bc9f8970ab3c208335f64564550b062c0d9eb6ple<std::pair<int, int>&&>, std::tuple<>>(std::_Rb_tree_const_iterator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::piecewise_construct_t const&, std::tuple<std::pair<int, int>&&>&&, std::tuple<>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_tree.h:2465:15 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x562822)
    #3 std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::operator[](std::pair<int, int>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_map.h:532:15 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c24c) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #4 amrex::ParticleContainer_impl<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator, amrex::DefaultAssignor>::DefineAndReturnParticleTile(int, int, int) /__w/WarpX/WarpX/build/_deps/fetchedamrex-src/Src/Particle/AMReX_ParticleContainer.H:1147:9 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x55c24c)
    #5 ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries(MultiParticleContainer&) (.omp_outlined_debug__) /__w/WarpX/WarpX/Source/Particles/ParticleBoundaryBuffer.cpp:399:61 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5540d2) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #6 ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries(MultiParticleContainer&) (.omp_outlined) /__w/WarpX/WarpX/Source/Particles/ParticleBoundaryBuffer.cpp:392:1 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5540d2)
    #7 __kmp_invoke_microtask <null> (libomp.so.5+0xdbb82) (BuildId: bc9f8970ab3c208335f64564550b062c0d9eb665)
    #8 ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries(MultiParticleContainer&) /__w/WarpX/WarpX/Source/Particles/ParticleBoundaryBuffer.cpp:392:26 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x553d75) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #9 WarpX::HandleParticlesAtBoundaries(int, float, int) /__w/WarpX/WarpX/Source/Evolve/WarpXEvolve.cpp:492:33 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x1849ba) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #10 WarpX::Evolve(int) /__w/WarpX/WarpX/Source/Evolve/WarpXEvolve.cpp:228:9 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x1807ff) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #11 main /__w/WarpX/WarpX/Source/main.cpp:40:15 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x139114) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #12 main /__w/WarpX/WarpX/Source/main.cpp:26:5 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x13909d) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)

  Location is heap block of size 48 at 0x7b0c0001d400 allocated by main thread:
    #0 operator new(unsigned long) <null> (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x13811b) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #1 std::__new_allocator<std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>>::allocate(unsigned long, void const*) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/new_allocator.h:147:27 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5573aa) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #2 std::allocator_traits<std::allocator<std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>>>::allocate(std::allocator<std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>>&, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/alloc_traits.h:482:20 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5573aa)
    #3 std::_Vector_base<std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>, std::allocator<std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>>>::_M_allocate(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:378:20 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5573aa)
    #4 std::vector<std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>, std::allocator<std::map<std::pair<int, int>, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>>>::reserve(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/vector.tcc:79:22 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5573aa)
    #5 amrex::ParticleContainer_impl<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator, amrex::DefaultAssignor>::reserveData() /__w/WarpX/WarpX/build/_deps/fetchedamrex-src/Src/Particle/AMReX_ParticleContainerI.H:299:17 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5659b7) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #6 amrex::ParticleContainer_impl<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator, amrex::DefaultAssignor>::ParticleContainer_impl(amrex::ParGDBBase*) /__w/WarpX/WarpX/build/_deps/fetchedamrex-src/Src/Particle/AMReX_ParticleContainer.H:221:33 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x5659b7)
    #7 amrex::ParticleContainer_impl<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator, amrex::DefaultAssignor> amrex::ParticleContainer_impl<amrex::SoAParticle<6, 0>, 6, 0, amrex::DefaultAllocator, amrex::DefaultAssignor>::make_alike<amrex::PinnedArenaAllocator>() const /__w/WarpX/WarpX/build/_deps/fetchedamrex-src/Src/Particle/AMReX_ParticleContainer.H:1334:37 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x563a2d) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #8 NamedComponentParticleContainer<amrex::PinnedArenaAllocator> NamedComponentParticleContainer<amrex::DefaultAllocator>::make_alike<amrex::PinnedArenaAllocator>() const /__w/WarpX/WarpX/Source/Particles/NamedComponentParticleContainer.H:143:87 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x563a2d)
    #9 ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries(MultiParticleContainer&) /__w/WarpX/WarpX/Source/Particles/ParticleBoundaryBuffer.cpp:380:36 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x553942) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #10 WarpX::HandleParticlesAtBoundaries(int, float, int) /__w/WarpX/WarpX/Source/Evolve/WarpXEvolve.cpp:492:33 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x1849ba) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #11 WarpX::Evolve(int) /__w/WarpX/WarpX/Source/Evolve/WarpXEvolve.cpp:228:9 (warpx.2d.MPI.OMP.SP65)
    #10 ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries(MultiParticleContainer&) /__w/WarpX/WarpX/Source/Particles/ParticleBoundaryBuffer.cpp:392:26 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x553d75) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)

  Previous write of size 8 at 0x7b5400007fa0 by main thread:
    #0 std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>::pair<std::pair<int, int>&&, 0ul>(std::tuple<std::pair<int, int>&&>&, std::tuple<>&, std::_Index_tuple<0ul>, std::_Index_tuple<>) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/tuple:2253:9 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x562750) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #1 std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>::pair<std::pair<int, int>&&>(std::piecewise_construct_t, std::tuple<std::pair<int, int>&&>, std::tuple<>) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/tuple:2241:9 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x562750)
    #2 void std::__new_allocator<std::_Rb_tree_node<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::construct<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::piecewise_construct_t const&, std::tuple<std::pair<int, int>&&>, std::tuple<>>(std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>*, std::piecewise_construct_t const&, std::tuple<std::pair<int, int>&&>&&, std::tuple<>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/new_allocator.h:187:23 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x562750)
    #3 void std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>>::construct<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::piecewise_construct_t const&, std::tuple<std::pair<int, int>&&>, std::tuple<>>(std::allocator<std::_Rb_tree_node<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>&, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>*, std::piecewise_construct_t const&, std::tuple<std::pair<int, int>&&>&&, std::tuple<>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/alloc_traits.h:537:8 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x562750)
    #4 void std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::_Select1st<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::_M_construct_node<std::piecewise_construct_t const&, std::tuple<std::pair<int, int>&&>, std::tuple<>>(std::_Rb_tree_node<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>*, std::piecewise_construct_t const&, std::tuple<std::pair<int, int>&&>&&, std::tuple<>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_tree.h:597:8 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x562750)
    #5 std::_Rb_tree_node<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>* std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaA.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x1807ff) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #12 main /__w/WarpX/WarpX/Source/main.cpp:40:15 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x139114) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #13 main /__w/WarpX/WarpX/Source/main.cpp:26:5 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x13909d) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)

  Thread T3 (tid=14686, running) created by main thread at:
    #0 pthread_create <null> (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0xaeebf) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #1 <null> <null> (libomp.so.5+0xb3eb3) (BuildId: bc9f8970ab3c208335f64564550b062c0d9eb665)
    #2 amrex::Initialize(int&, char**&, bool, ompi_communicator_t*, std::function<void ()> const&, std::ostream&, std::ostream&, void (*)(char const*)) /__w/WarpX/WarpX/build/_deps/fetchedamrex-src/Src/Base/AMReX.cpp:620:5 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x8263de) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #3 amrex::Initialize(int&, char**&, bool, ompi_communicator_t*, std::function<void ()> const&, std::ostream&, std::ostream&, void (*)(char const*)) /__w/WarpX/WarpX/build/_deps/fetchedamrex-src/Src/Base/AMReX.cpp:465:5 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x825aad) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #4 warpx::initialization::amrex_init(int&, char**&, bool) /__w/WarpX/WarpX/Source/Initialization/WarpXAMReXInit.cpp:70:16 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x28a24e) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)
    #5 main /__w/WarpX/WarpX/Source/main.cpp:26:5 (warpx.2d.MPI.OMP.SP.PSP.OPMD.PSATD.QED.GENQEDTABLES+0x13909d) (BuildId: f470761fd4dda67d3dbc566a37c719fc89b705e2)

SUMMARY: ThreadSanitizer: data race /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_tree.h:1034:24 in std::_Rb_tree<std::pair<int, int>, std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>, std::_Select1st<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>, std::less<std::pair<int, int>>, std::allocator<std::pair<std::pair<int, int> const, amrex::ParticleTile<amrex::SoAParticle<6, 0>, 6, 0, amrex::PinnedArenaAllocator>>>>::size() const

WeiqunZhang commented 4 months ago

I think this is a different issue from the issue in ParIter.

WeiqunZhang commented 4 months ago

Re: operator++, this might work

    void operator++ ()
    {
        m_particle_current_tile = nullptr;
        auto& particles = m_pc->GetParticles(m_level);
        while (true)
        {
            MFIter::operator++();
            if (isValid()) {
                auto key = std::make_pair(index(), LocalTileIndex());
                auto f = particles.find(key);
                if (f != particles.end() && f->second.numParticles() > 0) {
                    m_particle_current_tile = &(f->second);
                    break;
                }
            } else {
                break;
            }
        }
    }

WeiqunZhang commented 4 months ago

Re: the new issue reported by @lucafedeli88, I think the change needs to be made in WarpX like below. The root issue is DefineAndReturnParticleTile is not thread safe.

diff --git a/Source/Particles/ParticleBoundaryBuffer.cpp b/Source/Particles/ParticleBoundaryBuffer.cpp
index 345a0bfa5..9fbc332d4 100644
--- a/Source/Particles/ParticleBoundaryBuffer.cpp
+++ b/Source/Particles/ParticleBoundaryBuffer.cpp
@@ -387,17 +387,22 @@ void ParticleBoundaryBuffer::gatherParticlesFromDomainBoundaries (MultiParticleC
                 auto& species_buffer = buffer[i];
                 for (int lev = 0; lev < pc.numLevels(); ++lev)
                 {
+                    for (PIter pti(pc, lev); pti.isValid(); ++pti) {
+                        species_buffer.DefineAndReturnParticleTile(
+                            lev, pti.index(), pti.LocalTileIndex());
+                    }
+
                     const auto& plevel = pc.GetParticles(lev);
+
 #ifdef AMREX_USE_OMP
 #pragma omp parallel if (amrex::Gpu::notInLaunchRegion())
 #endif
                     for(PIter pti(pc, lev); pti.isValid(); ++pti)
                     {
                         auto index = std::make_pair(pti.index(), pti.LocalTileIndex());
-                        if(plevel.find(index) == plevel.end()) { continue; }

-                        auto& ptile_buffer = species_buffer.DefineAndReturnParticleTile(
-                                                        lev, pti.index(), pti.LocalTileIndex());
+                        auto& ptile_buffer = species_buffer.ParticlesAt
+                            (lev, pti.index(), pti.LocalTileIndex());
                         const auto& ptile = plevel.at(index);
                         auto np = ptile.numParticles();
                         if (np == 0) { continue; }

WeiqunZhang commented 4 months ago

@atmyers My suggestion on operator++ has a flaw. So it will not work. However, the flaw also exist in this PR. Suppose there are four tiles and a single box. Only one tile has particles. We have 4 threads. In the development branch, only one thread (say thread 0) will enter the loop body and the other three threads will fail isValid as expected. But in this PR, because operator++ is run after at least going through one loop iteration, three threads will execute loop body with no particles. I think we need to modify the isValid test too.

WeiqunZhang commented 4 months ago

Using this amrex branch https://github.com/WeiqunZhang/amrex/tree/fix_part_race_condition and this WarpX branch https://github.com/WeiqunZhang/WarpX/tree/test_fix_part_race_condition, the thread sanitizer no longer fails with race condition errors. It does fail eventually with MLMG failure below

2024-05-24T20:16:21.1393577Z STEP 2 starts ...
2024-05-24T20:16:21.2574426Z MLMG: # of AMR levels: 1
2024-05-24T20:16:21.2575045Z       # of MG levels on the coarsest AMR level: 7
2024-05-24T20:16:21.2593382Z MLMG: Initial rhs               = 796188480
2024-05-24T20:16:21.2594020Z MLMG: Initial residual (resid0) = 437492352
2024-05-24T20:16:21.2941102Z MLMG: Iteration   1 Fine resid/bnorm = 0.001869145315
2024-05-24T20:16:21.3261483Z MLMG: Iteration   2 Fine resid/bnorm = 9.022988706e-06
2024-05-24T20:16:21.3589663Z MLMG: Iteration   3 Fine resid/bnorm = 1.40670204e-06
2024-05-24T20:16:21.3922571Z MLMG: Iteration   4 Fine resid/bnorm = 1.848808438e-06
2024-05-24T20:16:21.4252564Z MLMG: Iteration   5 Fine resid/bnorm = 2.009574473e-06
2024-05-24T20:16:21.4576628Z MLMG: Iteration   6 Fine resid/bnorm = 1.446893634e-06
2024-05-24T20:16:21.4913793Z MLMG: Iteration   7 Fine resid/bnorm = 1.567468075e-06
2024-05-24T20:16:21.5249652Z MLMG: Iteration   8 Fine resid/bnorm = 1.446893634e-06
2024-05-24T20:16:21.5587989Z MLMG: Iteration   9 Fine resid/bnorm = 1.637803166e-06
2024-05-24T20:16:21.5927009Z MLMG: Iteration  10 Fine resid/bnorm = 1.40670204e-06
2024-05-24T20:16:21.6263505Z MLMG: Iteration  11 Fine resid/bnorm = 1.728233997e-06
2024-05-24T20:16:21.6588959Z MLMG: Iteration  12 Fine resid/bnorm = 1.326319079e-06
2024-05-24T20:16:21.6919761Z MLMG: Iteration  13 Fine resid/bnorm = 1.40670204e-06
2024-05-24T20:16:21.7255194Z MLMG: Iteration  14 Fine resid/bnorm = 1.326319079e-06
...
2024-05-24T20:16:27.7558465Z MLMG: Iteration 195 Fine resid/bnorm = 1.36651056e-06
2024-05-24T20:16:27.7896107Z MLMG: Iteration 196 Fine resid/bnorm = 1.36651056e-06
2024-05-24T20:16:27.8216529Z MLMG: Iteration 197 Fine resid/bnorm = 1.286127599e-06
2024-05-24T20:16:27.8545770Z MLMG: Iteration 198 Fine resid/bnorm = 1.173089117e-06
2024-05-24T20:16:27.8874770Z MLMG: Iteration 199 Fine resid/bnorm = 1.537324465e-06
2024-05-24T20:16:27.9221741Z amrex::Abort::1::MLMG failed. !!!
2024-05-24T20:16:27.9222623Z MLMG: Iteration 200 Fine resid/bnorm = 1.36651056e-06
2024-05-24T20:16:27.9223841Z SIGABRT
2024-05-24T20:16:27.9225693Z MLMG: Failed to converge after 200 iterations. resid, resid/bnorm = 1088, 1.36651056e-06
2024-05-24T20:16:27.9237822Z amrex::Abort::0::MLMG failed. !!!

But this is probably a different issue. Maybe the tolerance needs to be loosened.

WeiqunZhang commented 4 months ago

@atmyers My suggestion on operator++ has a flaw. So it will not work. However, the flaw also exist in this PR. Suppose there are four tiles and a single box. Only one tile has particles. We have 4 threads. In the development branch, only one thread (say thread 0) will enter the loop body and the other three threads will fail isValid as expected. But in this PR, because operator++ is run after at least going through one loop iteration, three threads will execute loop body with no particles. I think we need to modify the isValid test too.

No need to overload isValid. What we need is to call MFIter::opearator++ in the ctor to make sure it advances to the first non-empty particle tile. See my branch. Note that MFIter::operator++ is thread safe.

WeiqunZhang commented 4 months ago

Let's not merge this until after the next release.