Particle: Lift state array access outside of loops

[jeremylt]

Description and Context

This is the bones of my effort to lift particle state array access outside of loops. This is a prerequsite to GPU implementation. I'm not fully satisfied with the code, so this PR is currently in draft. Points of irritation

• The raw array access is prone to errors by coders, as I can personally attest. Some inline helper might be appropriate here? (inline so it doesn't disrupt GPU porting) Perhaps something like

static inline bundle_state_ptrs_index(ConstParticleContainerBundleStatePtrs& ptrs, ParticleType type, ParticleStatus status, int index, int statedim)
{
  return &ptrs[static_cast<int>(type)][static_cast<int>(status)][index * statedim];
}

with callsites looking like

-const double* mass_i = &mass[type_i_idx][status_i_idx][particle_i];
+const double* mass_i = Particle::bundle_state_ptrs_index(mass, type_i, status_i, particle_i, 1);
-const double* pos_i = &pos[type_i_idx][status_i_idx][particle_i * statedim];
+const double* pos_i = Particle::bundle_state_ptrs_index(pos, type_i, status_i, particle_i, statedim);

More verbose (though possibly fixable) but clearer intent.

• Similar annoyance with repeated logic for the replacement for try_*, which maybe could be

static inline try_bundle_state_ptrs_index(ConstParticleContainerBundleStatePtrs& ptrs, ParticleType type, ParticleStatus status, int index, int statedim, double *default = nullptr)
{
  return ptrs[static_cast<int>(type)][static_cast<int>(status)]
    ? &ptrs[static_cast<int>(type)][static_cast<int>(status)][index * statedim]
    : default;
}

with callsites looking like

-double* tempdot_i = tempdot[type_i_idx][status_i_idx]
-  ? &tempdot[type_i_idx][status_i_idx][particle_i]
-  : nullptr;
+double* tempdot_i = Particle::bundle_state_ptrs_index(tempdot, type_i,
+  status_i, particle_i, statedim);
-const double* dens_i = dens[type_i_idx][status_i_idx]
-  ? &dens[type_i_idx][status_i_idx][particle_i]
-  : &(basematerial_i->initDensity_);
+const double* dens_i = Particle::bundle_state_ptrs_index(dens, type_i, status_i,
+  status_i, particle_i, statedim, &(basematerial_i->initDensity_);

These two really are showing my stronger knowledge of C and weaker knowledge of C++ conventions (or families of conventions, as it may be).

• While (hopefully) the compiler should compile out these temp variables, I wonder if in some spots if it would be easier not to make foo_i and just use foo.

• There is the implicit (and I believe correct) assumption here that the first particle of a pair is always owned? Would be good to document somewhere if I can confirm this.

• On the topic of conventions, should I have ConstParticleContainerBundleStatePtrs& or auto& (and ParticleContainerBundleStatePtrs& and auto&?). I created rather verbose type aliases here but I'm not sure of a shorter while also clear name? In any case, I do see usage of auto but not sure when its preferred and not.

• Need to profile this. I do wonder if it helps perf.

Edit: Added the bundle_state_ptrs_index helper and it definitely makes intent clearer to read imho. For the moment I'm keeping with the convention of the local variables foo_i, as those should be compiled out. I have not switched to auto& at the moment, but that's an easy swap if desired. Profiling is a ToDo I'm curious about.

Related Issues and Pull Requests

See long term plan in #2148

Disclosure of AI assistance

None

[jeremylt]

note to me - issue of requesting ptrs to containers with 0 particles

[jeremylt]

Couple of failing local tests on last commit, need to investigate after lunch

[jeremylt]

Ugh, some of the CI compilers are less capable of understanding overloading than my local machine

[jeremylt]

Ok, logic is straightened out - need to benchmark now and see if this has any perf changes. Optimistically, I'd hope it helps