JuliaMolSim / DFTK.jl

Density-functional toolkit
https://docs.dftk.org
MIT License
437 stars 89 forks source link

Should `Model` directly subtype `AbstractSystem`? #1007

Open rkurchin opened 2 weeks ago

rkurchin commented 2 weeks ago

I'm going through some things to prep my AtomsBase introductory materials for the MolSSI workshop and noticed that at the moment, DFTK provides functions to convert to prototype interface implementations, but why not just (also, or instead) have Model directly implement the interface and subtype AbstractSystem? I could see some advantages down the line where if you had to pass information back and forth quite often, you wouldn't want to repeatedly incur the computational cost of conversion.

It's certainly not a blocker for any particular thing I'm doing right now, but figured I'd start the conversation as it doesn't seem it would be too difficult to do. I'd be happy to draft a PR if this makes sense!

antoine-levitt commented 2 weeks ago

Just had a look at the interface, and it seems OK, with the caveat that model is not mutable (and that there is no notion of velocity)

mfherbst commented 2 weeks ago

I did this originally when getting atomsbase support up, but @antoine-levitt vetoed it at the time :smile:, so no blockers from me.

antoine-levitt commented 2 weeks ago

Did I really? I guess I didn't want to commit to atomsbase at the time (model can only subtype once), but now it's relatively established. I still don't really see what it buys either us or downstream consumers (Model is just a container and has no specific computational cost, so conversions are just copies of pointers, and if you're worried about that cost you probably shouldn't do DFT :-)), but if there's demand why not...