Luthaf / rascaline

Computing representations for atomistic machine learning
https://luthaf.fr/rascaline/
BSD 3-Clause "New" or "Revised" License
44 stars 13 forks source link

SphericalBesselBasis.compute_zeros returns incorrect shape #288

Closed sirmarcel closed 7 months ago

sirmarcel commented 7 months ago

Hello,

I've noticed that the compute_zeros function in SphericalBesselBasis returns incorrect shapes:

from rascaline.utils import SphericalBesselBasis

cutoff = 4.4
max_angular = 10
max_radial = 10

basis = SphericalBesselBasis(cutoff, max_radial, max_angular)

assert basis.compute_zeros(max_angular, max_radial).shape == (11, 11)

fails, because the returned shape is (11, 10). Given that both max_angular and max_radial are supposed to give maximum values, I would expect the shape to be (11, 11).

This should be an easy fix, but I'm not sure if there's some subtlety with the zeroes of the Bessel function that make it more complex. Maybe @kvhuguenin can weigh in.

Luthaf commented 7 months ago

max_radial is exclusive while max_angular is inclusive. This is a leftover from librascal that is there everywhere in rascaline

sirmarcel commented 7 months ago

Oh my, okay. I guess this is a wontfix then, I'll close it. Thanks!

Luthaf commented 7 months ago

This is wontfix for now yes. We might update this to be less confusing whenever we get around updating all hypers in rascaline!