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

Coop coordination #280

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

Coop coordination #280

wants to merge 12 commits into from

Conversation

juliette1996
Copy link
Contributor

No description provided.

Add an optional key to the dictionary input of COOP calculation that includes the coordination number to be considered for each of the two elements
from qmflows.parsers.xyzParser import readXYZ

from nanoCAT.recipes import coordination_number
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nanoCAT must be included in the library requirements at setup.py

@felipeZ
Copy link
Contributor

felipeZ commented Sep 11, 2020

@juliette1996 Could you please provide a description of what is the purpose of this PR?

@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2020

Codecov Report

Merging #280 into master will decrease coverage by 0.39%.
The diff coverage is 42.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #280      +/-   ##
==========================================
- Coverage   83.23%   82.84%   -0.40%     
==========================================
  Files          23       23              
  Lines        1563     1574      +11     
  Branches      194      198       +4     
==========================================
+ Hits         1301     1304       +3     
- Misses        209      215       +6     
- Partials       53       55       +2     
Impacted Files Coverage Δ
nanoqm/workflows/schemas.py 97.36% <ø> (ø)
nanoqm/workflows/workflow_coop.py 87.69% <42.85%> (-12.31%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f4d2a2...6058891. Read the comment docs.

element_2_index = [i for i, s in enumerate(mol) if element_2.lower() in s]

# For the two selected elements, only the indices corresponding to a given coordination number
else:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fyi, you can also make nano-CAT an optional dependency and then raise any potential ImportErrors whenever a code branch is reached that does require nano-CAT:

from typing import Optional
try:
    from nanoCAT import coordination_number
    NANOCAT_EX: Optional[ImportError] = None
except ImportError as ex:
    NANOCAT_EX = ex


def compute_overlap_and_atomic_orbitals(...):
    if config["elements_coordination"] is None:
        ...
    elif NANOCAT_EX is not None:
        raise NANOCAT_EX
    else:
        coord = coordination_number(geometry)
        ...

@felipeZ
Copy link
Contributor

felipeZ commented Sep 25, 2020

Hi @juliette1996 is there any update in this PR? :)

@BvB93 BvB93 force-pushed the master branch 8 times, most recently from 4e68969 to 6830e71 Compare September 13, 2023 12:50
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.

4 participants