PHAREHUB / PHARE

💫 Parallel Hybrid Particle In Cell code with Adaptive mesh REfinement
https://phare.readthedocs.io
GNU General Public License v3.0
71 stars 24 forks source link

zero particle to pack #860

Open nicolasaunai opened 4 months ago

nicolasaunai commented 4 months ago

Possible bug hitting the assertion:

TBOX_ASSERT(num_bytes >= 1); in MessageStream.cpp:50

in run058 where there are two populations with density falling below density CutOff (i.e. no particles). I suspect this comes from


        std::size_t getDataStreamSize(SAMRAI::hier::BoxOverlap const& overlap) const override
        {
            auto const& pOverlap{dynamic_cast<SAMRAI::pdat::CellOverlap const&>(overlap)};

            return countNumberParticlesIn_(pOverlap) * sizeof(Particle_t);
        }

which can return 0 when the patch has no particle for the population of which the particleData is from.

There may be a way to have SAMRAI dealing with "empty patch data". Another and maybe more efficient solution might come from realizing that, at the moment, we can't handle vacuum and thus there should not be any cell with 0 particles. Therefore if we were to send all populations at once the problem would never arise. Side effect would be we would send only one message instead of one per pop.

Pack/Copy operations are per ParticleData, which is per Population at the moment. So grouping populations means that we should rather have them all in a ParticleData.

something like replacing

        ParticleArray domainParticles;
        ParticleArray patchGhostParticles;

        ParticleArray levelGhostParticles;

        ParticleArray levelGhostParticlesOld;
        ParticleArray levelGhostParticlesNew;

        core::ParticlesPack<ParticleArray> pack;

by

       std::vector<ParticleArray> domainParticles;
       std::vector<ParticleArray> patchGhostParticles;

       std::vector<ParticleArray> levelGhostParticles;

       std::vector<ParticleArray> levelGhostParticlesOld;
       std::vector<ParticleArray> levelGhostParticlesNew;

        std::vector<core::ParticlesPack<ParticleArray> >pack;

the vectors would be for handling populations. Then in IonPopulation would not have its ParticlePack assigned to that of a specific ParticleData, but to that of a specific item of the std::vector pack.

This will also change how schedules are created since only one should exist now for the now single ParticleData objet per patch.

Note: this may also simply the runtimeResourceUserList thing since now there would ever be only one ParticleData and not a runtime known number of them.

PhilipDeegan commented 2 months ago

given we always prepend a single size_t to the bytestream to indicate the number of particles in the stream, could a simple solution not be to just change getDataStreamSize to be

        std::size_t getDataStreamSize(SAMRAI::hier::BoxOverlap const& overlap) const override
        {
            auto const& pOverlap{dynamic_cast<SAMRAI::pdat::CellOverlap const&>(overlap)};

            return sizeof(std::size_t) +  (countNumberParticlesIn_(pOverlap) * sizeof(Particle_t) );
        }
PhilipDeegan commented 2 months ago

image