Cutoff functions
A couple of issues on cutoff functions:
-
The
gamma
keyword on thePolynomial
cutoff is not actually used, at least in theGaussian
descriptor. In the current code, the cutoff is broken down into a name and an Rc, then put back together in functions likecalculate_G2
. -
The current implementation breaks
Amp.load
, as all keys inself.parameters
need to be keyword arguments to the descriptor. Currently there is something calledself.parameters.cutofffn
which breaks it.
I am putting in a todict
method into our cutoffs, and a dict2cutoff
function. This is modeled after how ASE does constraints.
This still will leave open the issue of how a user can supply a custom cutoff, which we need to address.
Comments (12)
-
reporter -
reporter This is now partially resolved in commit c40f6c4. It is no longer a blocker on using
Amp.load
. However, I think we still have to think about three issues:- Because gamma was being ignored for the Polynomial cutoff, I just added a NotImplementedError if one tries to use anything but the Cosine cutoff. This should be simple enough to fix, but the fmodules needs to be tweaked.
- The user still can't really specify their own cutoff function, if this is our intention. I don't know if it's important that they be able to. But we may want to think about this.
- I only fixed this in the
Gaussian
descriptor; this will need to be propagated to theBispetrum
andZernike
.
We should also get a test in for
Amp.load
so that it isn't accidentally broken again... I'll create a separate issue for that. -
In the manuscript we showed that the polynomial cutoff approximately-well reduces to the cosine cutoff for \gamma = 2. What about totally getting ride of the cosine function accordingly?
-
reporter I don't think that saves us much -- right now the Polynomial is the one that is broken and needs to be fixed.
-
I am looking inside this report.
-
I would vote for getting ride of the Cosine function completely, and just caring about the polynomial one. The polynomial cutoff is the most general form which has the least-required properties for a cutoff function without which systematic bug exists.
-
I would say that keeping both would be nicer, and if wanted, set Polynomial as the default?. It is good to give users more options when performing calculations. What do you think?.
-
@akhorshi I have checked now Fig. 5 when γ = 2. I see what you mean. That also could be set as the default if it is decided to get rid of the Cosine function.
-
reporter @akhorshi What do you mean that " without which systematic bug exists."?
-
Like we discussed the other day, polynomial cutoff has four constraints: zero value and zero slope at the cutoff boundary; unit value and zero slope at r=0. The first two are mandatory and should apply to any function the user designs (otherwise the energy or force field is not continuous). The second two have no practical importance because they are at r=0. So I don't see any reason the user wants a different behavior at r=0.
In between r=0 and r=R_c, the polynomial can diminish with any user-specified rate. Therefore, I assume it can well-approximate any behavior the user may like.
-
I have uploaded a first commit aimed to fix this issue in a new branch called issue66. I need to still work on
Bispectrum
for which the Fortran code has to be done from scratch. -
- changed status to resolved
- Log in to comment
We may ultimately have to have a getter/setter so that the user can set the cutoff, which will then also set it in p.cutoff as a dictionary...