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

Nvalence_modification #290

Merged
merged 3 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions smact/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ class Element:

Element.num_valence (int): Number of valence electrons

Element.num_valence_modified (int): Number of valence electrons based on a modified definition



Raises:
Expand Down Expand Up @@ -162,6 +164,12 @@ def __init__(
MeltingT = None
num_valence = None

valence_data = data_loader.lookup_element_valence_data(symbol)
if valence_data:
num_valence_modified = valence_data["NValence"]
else:
num_valence_modified = None
Comment on lines +167 to +171
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor data retrieval and setting for num_valence_modified.

The data retrieval process using data_loader.lookup_element_valence_data(symbol) and the subsequent setting of num_valence_modified are correctly implemented. However, consider using the ternary operator for cleaner code as suggested by static analysis.

-        if valence_data:
-            num_valence_modified = valence_data["NValence"]
-        else:
-            num_valence_modified = None
+        num_valence_modified = valence_data["NValence"] if valence_data else None
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
valence_data = data_loader.lookup_element_valence_data(symbol)
if valence_data:
num_valence_modified = valence_data["NValence"]
else:
num_valence_modified = None
valence_data = data_loader.lookup_element_valence_data(symbol)
num_valence_modified = valence_data["NValence"] if valence_data else None
Tools
Ruff

168-171: Use ternary operator num_valence_modified = valence_data["NValence"] if valence_data else None instead of if-else-block

Replace if-else-block with num_valence_modified = valence_data["NValence"] if valence_data else None

(SIM108)


for attribute, value in (
("coord_envs", coord_envs),
("covalent_radius", dataset["r_cov"]),
Expand Down Expand Up @@ -200,6 +208,7 @@ def __init__(
("AtomicWeight", AtomicWeight),
("MeltingT", MeltingT),
("num_valence", num_valence),
("num_valence_modified", num_valence_modified),
# ('vdw_radius', dataset['RVdW']),
):
setattr(self, attribute, value)
Expand Down
98 changes: 98 additions & 0 deletions smact/data/element_valence_modified.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
element,NValence
H,1
He,2
Li,1
Be,2
B,3
C,4
N,5
O,6
F,7
Ne,8
Na,1
Mg,2
Al,3
Si,4
P,5
S,6
Cl,7
Ar,8
K,1
Ca,2
Sc,3
Ti,4
V,5
Cr,6
Mn,7
Fe,8
Co,9
Ni,10
Cu,11
Zn,12
Ga,3
Ge,4
As,5
Se,6
Br,7
Kr,8
Rb,1
Sr,2
Y,3
Zr,4
Nb,5
Mo,6
Tc,7
Ru,8
Rh,9
Pd,10
Ag,11
Cd,12
In,3
Sn,4
Sb,5
Te,6
I,7
Xe,8
Cs,1
Ba,2
La,3
Ce,4
Pr,5
Nd,6
Pm,7
Sm,8
Eu,9
Gd,10
Tb,11
Dy,12
Ho,13
Er,14
Tm,15
Yb,16
Lu,3
Hf,4
Ta,5
W,6
Re,7
Os,8
Ir,9
Pt,10
Au,11
Hg,12
Tl,3
Pb,4
Bi,5
Po,6
At,7
Rn,8
Fr,1
Ra,2
Ac,3
Th,4
Pa,5
U,6
Np,7
Pu,8
Am,9
Cm,10
Bk,11
51 changes: 51 additions & 0 deletions smact/data_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -904,3 +904,54 @@
)

return None


_element_valence_data = None


def lookup_element_valence_data(symbol: str, copy: bool = True):
"""
Retrieve valence electron data.

For d-block elements, the s and d electrons contribute to NValence.
For p-block elements, the s and p electrons contribute to NValence.
For s- and f-block elements, NValence is calculated from the Noble Gas electron configuration
i.e.

Args:
symbol : the atomic symbol of the element to look up.
copy: if True (default), return a copy of the data dictionary,
rather than a reference to a cached object -- only use
copy=False in performance-sensitive code and where you are
certain the dictionary will not be modified!

Returns:
NValence (int): the number of valence electrons
Returns None if the element was not found among the external
data.
"""

global _element_valence_data

if _element_valence_data is None:
_element_valence_data = {}

df = pd.read_csv(
os.path.join(data_directory, "element_valence_modified.csv")
)
for _index, row in df.iterrows():
key = row.iloc[0]

dataset = {"NValence": int(row.iloc[1])}
_element_valence_data[key] = dataset

if symbol in _element_valence_data:
return _element_valence_data[symbol]
else:
if _print_warnings:
print(

Check warning on line 952 in smact/data_loader.py

View check run for this annotation

Codecov / codecov/patch

smact/data_loader.py#L952

Added line #L952 was not covered by tests
"WARNING: Valence data for element {} not "
"found.".format(symbol)
)

return None
Comment on lines +909 to +957
Copy link
Contributor

Choose a reason for hiding this comment

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

Review of new function lookup_element_valence_data.

The function lookup_element_valence_data is well-documented and correctly implemented with caching mechanics. However, consider improving the error message to provide more context about why the data might be missing, such as a missing or malformed CSV file.

-            print(
-                "WARNING: Valence data for element {} not "
-                "found.".format(symbol)
-            )
+            print(f"WARNING: Valence data for element {symbol} not found. Check if 'element_valence_modified.csv' is present and correctly formatted.")
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_element_valence_data = None
def lookup_element_valence_data(symbol: str, copy: bool = True):
"""
Retrieve valence electron data.
For d-block elements, the s and d electrons contribute to NValence.
For p-block elements, the s and p electrons contribute to NValence.
For s- and f-block elements, NValence is calculated from the Noble Gas electron configuration
i.e.
Args:
symbol : the atomic symbol of the element to look up.
copy: if True (default), return a copy of the data dictionary,
rather than a reference to a cached object -- only use
copy=False in performance-sensitive code and where you are
certain the dictionary will not be modified!
Returns:
NValence (int): the number of valence electrons
Returns None if the element was not found among the external
data.
"""
global _element_valence_data
if _element_valence_data is None:
_element_valence_data = {}
df = pd.read_csv(
os.path.join(data_directory, "element_valence_modified.csv")
)
for _index, row in df.iterrows():
key = row.iloc[0]
dataset = {"NValence": int(row.iloc[1])}
_element_valence_data[key] = dataset
if symbol in _element_valence_data:
return _element_valence_data[symbol]
else:
if _print_warnings:
print(
"WARNING: Valence data for element {} not "
"found.".format(symbol)
)
return None
print(f"WARNING: Valence data for element {symbol} not found. Check if 'element_valence_modified.csv' is present and correctly formatted.")