Cutoff functions

Issue #66 resolved
andrew_peterson repo owner created an issue

A couple of issues on cutoff functions:

  • The gamma keyword on the Polynomial cutoff is not actually used, at least in the Gaussian descriptor. In the current code, the cutoff is broken down into a name and an Rc, then put back together in functions like calculate_G2.

  • The current implementation breaks Amp.load, as all keys in self.parameters need to be keyword arguments to the descriptor. Currently there is something called self.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)

  1. andrew_peterson reporter

    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...

  2. andrew_peterson 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:

    1. 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.
    2. 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.
    3. I only fixed this in the Gaussian descriptor; this will need to be propagated to the Bispetrum and Zernike.

    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.

  3. Alireza Khorshidi

    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?

  4. andrew_peterson reporter

    I don't think that saves us much -- right now the Polynomial is the one that is broken and needs to be fixed.

  5. Alireza Khorshidi

    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.

  6. Muammar El Khatib

    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?.

  7. Muammar El Khatib

    @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.

  8. Alireza Khorshidi

    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.

  9. Muammar El Khatib

    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.

  10. Alireza Khorshidi

    Fortran cutoffs (both cosine and polynomial) were added in a series of commits starting from 5f9d044 and ending with 7564eaf. Polynomial cutoff should now work well without crashing.

  11. Log in to comment