search

disordered-photonics / celes

CELES: CUDA-accelerated electromagnetic scattering by large ensembles of spheres
Other
48 stars 18 forks source link

Bug in `compute_maximal_particle_distance` #22

Closed AmosEgel closed 6 years ago

AmosEgel commented 6 years ago

The try-catch logic in compute_maximal_particle_distance (celes_particles.m) doesn't make sense. Got the error: "Undefined function or variable 'CH'."

lpattelli commented 6 years ago

can you check if commit 3ce1942 on the develop branch solves this problem?

AmosEgel commented 6 years ago

I don't think so. In the lowest catch block, CH is used - but if convhull fails, CH is not available.

lpattelli commented 6 years ago

you are right. the proposed code should now be able to correctly handle the case of collinear/coplanar particles, the case of not having pdist available, and both these cases happening at once.

I spent some time trying to come up with an efficient drop-in replacement for pdist, but then realized that this is probably an overkill. in most cases we can just use a reasonable overestimation of maxParticleDistance calculated as sqrt(sum((min(pos)-max(pos)).^2)). actually, it would be probably OK to just use this simple line of code to replace compute_maximal_particle_distance altogether, if you feel like to.

AmosEgel commented 6 years ago

Nice, this is the diagonal of a cuboid containing all particles, right?

lpattelli commented 6 years ago

that's exactly it. worst case scenario, we are overestimating maxParticleDistance by some +70% and the computational cost remains negligible for arbitrary number of particles.