JuliaMolSim / AtomsBase.jl

A Julian abstract interface for atomic structures.
https://juliamolsim.github.io/AtomsBase.jl/
MIT License
81 stars 16 forks source link

WIP: Prototypes for Calculators #86

Closed cortner closed 8 months ago

cortner commented 9 months ago

This PR goes with #84 - second attempt - for the time being we propose the following:

Comments:

Balancing the needs of classical (fast) FFs, MLFFs and electronic structure is a bit tricky which might lead to some strange looking choices initially. Please feel free to comment.

rkurchin commented 9 months ago

As I remarked at https://github.com/JuliaMolSim/AtomsBase.jl/issues/84#issuecomment-1743354964, I like this general idea – however, I do wonder if the functionality is distinct enough from AtomsBase, which so far has focused pretty exclusively on specifying structures, that it should actually be a separate package.

cortner commented 9 months ago

I'd be happy with that too, but then that's another package to maintain and keep compatible with AtomsBase. My suggestion is to keep it here for now but if it grows too much or if it becomes invonvenient in any way to have the two linked in a single package then this can just be split off into a new package.

I want to make progress towards some of the potential demos we discussed.

rkurchin commented 9 months ago

Yeah that's fair enough – I suppose the flexibility of Julian interfaces means that people could certainly implement only part of the interface in their package if it's truly not applicable for them.

cortner commented 9 months ago

I think the main debate is what the recommended approach should be

mfherbst commented 9 months ago

I'd be happy with that too, but then that's another package to maintain and keep compatible with AtomsBase. My suggestion is to keep it here for now but if it grows too much or if it becomes invonvenient in any way to have the two linked in a single package then this can just be split off into a new package.

Also linked to my comment in #84, but I agree that it's fine to have the stubs here at first and only factor out later if that's needed.

cortner commented 9 months ago

so the only question for now then is whether the experimental interface is fine as is or whether it needs more discussion.

tjjarvinen commented 9 months ago

Here is first draft.

There is documentation file calculations-interface.md that has details written out.

But basically you only need to define potential-energy, forces or forces! and virial to get the whole interface. You can customize for energy_forces_viral on top of that too.

The main point that in my opinion is important that we supply test functions to test the calculators, which I did.

There is still stuff that is missing. But you should already get the idea hot it works. Here is a small script to demonstrate

using AtomsBase
using Unitful
using UnitfulAtomic

struct MyType
end

struct MyOtherType
end

##

function AtomsBase.potential_energy(system, calculator::MyType; kwargs...)
    # we can ignore kwargs... or use them to tune the calculation
    # or give extra information like pairlist

    # add your own definition here
    return 0.0u"eV"
end

function AtomsBase.virial(system, calculator::MyType; kwargs...)
    # we can ignore kwargs... or use them to tune the calculation
    # or give extra information like pairlist

    # add your own definition here
    return zeros(3,3) * u"eV*Å"
end

AtomsBase.@generate_complement function AtomsBase.forces(system, calculator::Main.MyType; kwargs...)
    # we can ignore kwargs... or use them to tune the calculation
    # or give extra information like pairlist

    # add your own definition
    return zeros(AtomsBase.default_force_eltype, length(system)) 
end

AtomsBase.@generate_complement function AtomsBase.forces!(f::AbstractVector, system, calculator::Main.MyOtherType; kwargs...)
    @assert length(f) == length(system)
    # we can ignore kwargs... or use them to tune the calculation
    # or give extra information like pairlist

    # add your own definition
    for i in eachindex(f)
        f[i] = zero(AtomsBase.default_force_eltype)
    end

    return f
end

##

hydrogen = isolated_system([
    :H => [0, 0, 1.]u"bohr",
    :H => [0, 0, 3.]u"bohr"
])

AtomsBase.test_potential_energy(hydrogen, MyType())
AtomsBase.test_forces(hydrogen, MyType())
AtomsBase.test_virial(hydrogen, MyType())

AtomsBase.test_forces(hydrogen, MyOtherType())

##
AtomsBase.test_potential_energy(hydrogen, MyOtherType())  # this will fail
AtomsBase.test_virial(hydrogen, MyOtherType())  # this will fail

##

AtomsBase.potential_energy(hydrogen, MyType())
AtomsBase.forces(hydrogen, MyType())
AtomsBase.virial(hydrogen, MyType())
AtomsBase.energy_forces(hydrogen, MyType())
AtomsBase.energy_forces_virial(hydrogen, MyType())
cortner commented 8 months ago

Ready to close this?

tjjarvinen commented 8 months ago

From my part yes

cortner commented 8 months ago

Closing since this has been moved to AtomsCalculators.jl