openkim / kliff

KIM-based Learning-Integrated Fitting Framework for interatomic potentials.
https://kliff.readthedocs.io
GNU Lesser General Public License v2.1
34 stars 20 forks source link

ValueError call wrong when using energies only #1

Closed SouthKoreaLN closed 5 years ago

SouthKoreaLN commented 5 years ago

In loss.py, you have the error call at the end of this block.

I don't think it can coexist with option 1 where S=1 and not S=3N+1...

    1) If `use_energy == True` and `use_forces == False`, then `S = 1`.
    `prediction[0]` is the potential energy computed by the calculator, and
    `reference[0]` is the reference energy.

    2) If `use_energy == False` and `use_forces == True`, then `S = 3N`.
    `prediction[3*i+0]`, `prediction[3*i+1]`, and `prediction[3*i+2]` are the
    x, y, and z component of the forces on atom i in the configuration, respecrively.
    Correspondingly, `reference` is the 3N concatenated reference forces.

    3) If `use_energy == True` and `use_forces == True`, then `S = 3N + 1`.
    `prediction[0]` is the potential energy computed by the calculator, and
    `reference[0]` is the reference energy.
    `prediction[3*i+1]`, `prediction[3*i+2]`, and `prediction[3*i+3]` are the
    x, y, and z component of the forces on atom i in the configuration, respecrively.
    Correspondingly, `reference` is the 3N concatenated reference forces.
    """

    if len(prediction) != 3 * natoms + 1:
        raise ValueError("len(prediction) != 3N+1, where N is the number of atoms.")
mjwen commented 5 years ago

@nleconte Thanks for catching this!

A new version v0.1.2 was just released. It includes a fix of this and many other improvements. You can try this new version. You can check out the branch tagged v0.1.2, or simply it is the master branch (at least now).

As usually, please do not hesitate to report bugs here if you find any.