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

preliminary test for 0-orbital Atoms #733

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

zerothi
Copy link
Owner

@zerothi zerothi commented Mar 26, 2024

@tfrederiksen @pfebrer

this is a first stab at 449. I am starting to grow on the idea.

However, as you can see in this PR, there are many places in the code base where it assumes that there exists some orbitals on atoms.

Whether or not these should be defaulted to be present or not is something that needs to be discussed.
However, it seems that one should be explicit on defining orbitals if one wishes to use them.
Many places the code simply fails because it now requires some kind of explicitness.

But I would agree that this could be useful. If anybody cares to expand/fix/add tests for the remaining details, feel free :)

  • Closes Atoms without orbitals? #449
  • Added tests for new/changed functions?
  • Ran isort . and black . [24.2.0] at top-level
  • Documentation for functionality in docs/
  • Changes documented in CHANGELOG.md

@zerothi zerothi marked this pull request as draft April 10, 2024 10:22
@pfebrer
Copy link
Contributor

pfebrer commented May 2, 2024

I want to push this forward to use it for QM/MM geometries, should I start a new branch and create a separate PR?

@tfrederiksen
Copy link
Contributor

I haven't been looking much into this (sorry), so from my side you can, of course, proceed as you see fit. I'm still very much interested in this development in sisl!

@zerothi
Copy link
Owner Author

zerothi commented May 2, 2024

I want to push this forward to use it for QM/MM geometries, should I start a new branch and create a separate PR?

Yes, please do, branch this off would be ideal imho.

However, I think it would be very beneficial to discuss here, or reiterate the decisions made in #449. Eg how sparse matrices etc should handle things.

@zerothi
Copy link
Owner Author

zerothi commented May 2, 2024

Or perhaps better, finish the discussions in #449, then let implementation details be discussed in the pr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Atoms without orbitals?
3 participants