Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should Model directly subtype AbstractSystem? #1007

Open
rkurchin opened this issue Oct 14, 2024 · 3 comments · May be fixed by #1011
Open

Should Model directly subtype AbstractSystem? #1007

rkurchin opened this issue Oct 14, 2024 · 3 comments · May be fixed by #1011
Labels
discussion Discussion thread of broader scope

Comments

@rkurchin
Copy link

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!

@rkurchin rkurchin added the discussion Discussion thread of broader scope label Oct 14, 2024
@antoine-levitt
Copy link
Member

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
Copy link
Member

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

@antoine-levitt
Copy link
Member

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

@rkurchin rkurchin linked a pull request Oct 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion thread of broader scope
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants