Fermi.jl
Fermi.jl copied to clipboard
Non-concrete `Atom` types in `Molecule`
https://github.com/FermiQC/Fermi.jl/blob/721b55fe513cda2c79e5773bf950f00caeb66f89/src/Core/Molecule.jl#L64
I was browsing your repo and noticed that the Atom typed defined in Molecules.jl is parametric in {I,F}: but here, this parametric dependence is not accounted for.
As a result, any access of the elements of the atoms field's elements will be type-unstable.
Hey! Thanks for pointing it out. I don't know how I missed your issue until now. We have dropped this parametric dependency in the Atom struct. Now it simply reads as:
struct Atom
Z
mass
xyz
end
Found in: https://github.com/FermiQC/Molecules.jl/blob/6e32b19d16d8052fc613461ed60d61733fa3254c/src/Molecules.jl#L19
Hi @gustavojra. That change has some significant side-effects though: now, every access of a field of an Atom is type-unstable. Effectively, this will make anything that uses an Atom type-unstable.
It seems the right fix is to keep Atom parametric and simply make any struct containing an Atom parametric as well.
Ok, I think I misunderstood your first comment. We removed the type declarations so that we could use Atom with AutoDiff routines, but now I see how that can take a toll on the overall performance. I am changing Atom and Molecule to
struct Atom{T<:Real, X<:Real}
Z::Int
mass::T
xyz::SVector{3, X}
end
struct Molecule{A<:Atom}
atoms::Vector{A}
charge::Int
multiplicity::Int
Nα::Int
Nβ::Int
end
Hopefully, that solves the problem. Unless T and X must appear as parameters for Molecule as well.