-
Notifications
You must be signed in to change notification settings - Fork 31
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
Develop #83
Develop #83
Changes from all commits
8842607
de0091e
a3672c1
09862a2
2aa0816
3401b6b
30290e1
d0b7fb8
30a4255
97ddfcd
3752365
015cb5f
bd51d66
7e1ca00
edbed6d
1fd036f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -920,12 +920,14 @@ def formation_energy( | |
provided/present in format generated by ``doped`` (see tutorials). | ||
(Default: None) | ||
vbm (float): | ||
VBM eigenvalue in the bulk supercell, to use as Fermi level reference | ||
point for calculating formation energy. If None (default), will use | ||
"vbm" from the calculation_metadata dict attribute if present. | ||
VBM eigenvalue to use as Fermi level reference point for calculating | ||
formation energy. If ``None`` (default), will use ``"vbm"`` from the | ||
``calculation_metadata`` dict attribute if present -- which corresponds | ||
to the VBM of the `bulk supercell` calculation by default, unless | ||
``bulk_band_gap_vr`` is set during defect parsing). | ||
fermi_level (float): | ||
Value corresponding to the electron chemical potential, | ||
referenced to the VBM. Default is 0 (i.e. the VBM). | ||
Value corresponding to the electron chemical potential, referenced | ||
to the VBM eigenvalue. Default is 0 (i.e. the VBM). | ||
|
||
Returns: | ||
Formation energy value (float) | ||
|
@@ -1108,9 +1110,11 @@ def equilibrium_concentration( | |
temperature (float): | ||
Temperature in Kelvin at which to calculate the equilibrium concentration. | ||
vbm (float): | ||
VBM eigenvalue in the bulk supercell, to use as Fermi level reference | ||
point for calculating formation energy. If None (default), will use | ||
"vbm" from the calculation_metadata dict attribute if present. | ||
VBM eigenvalue to use as Fermi level reference point for calculating | ||
the formation energy. If ``None`` (default), will use ``"vbm"`` from the | ||
``calculation_metadata`` dict attribute if present -- which corresponds | ||
to the VBM of the `bulk supercell` calculation by default, unless | ||
``bulk_band_gap_vr`` is set during defect parsing). | ||
Comment on lines
+1113
to
+1117
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarify the VBM handling in the Similar to the |
||
fermi_level (float): | ||
Value corresponding to the electron chemical potential, | ||
referenced to the VBM. Default is 0 (i.e. the VBM). | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1048,7 +1048,7 @@ def __init__( | |
interstitial_coords: Optional[list] = None, | ||
generate_supercell: bool = True, | ||
charge_state_gen_kwargs: Optional[dict] = None, | ||
supercell_gen_kwargs: Optional[dict] = None, | ||
supercell_gen_kwargs: Optional[dict[str, Union[int, float, bool]]] = None, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure proper type hinting for the - supercell_gen_kwargs: Optional[dict[str, Union[int, float, bool]]] = None,
+ supercell_gen_kwargs: Optional[Dict[str, Union[int, float, bool]]] = None, Please import
|
||
interstitial_gen_kwargs: Optional[dict] = None, | ||
target_frac_coords: Optional[list] = None, | ||
processes: Optional[int] = None, | ||
|
@@ -1201,12 +1201,19 @@ class (such as ``clustering_tol``, ``stol``, ``min_dist`` etc), or to | |
self.charge_state_gen_kwargs = ( | ||
charge_state_gen_kwargs if charge_state_gen_kwargs is not None else {} | ||
) | ||
self.supercell_gen_kwargs = supercell_gen_kwargs if supercell_gen_kwargs is not None else {} | ||
self.interstitial_gen_kwargs = ( | ||
self.supercell_gen_kwargs: dict[str, Union[int, float, bool]] = { | ||
"min_image_distance": 10.0, # same as current pymatgen-analysis-defects `min_length` ( = 10) | ||
"min_atoms": 50, # different from current pymatgen-analysis-defects `min_atoms` ( = 80) | ||
"ideal_threshold": 0.1, | ||
"force_cubic": False, | ||
"force_diagonal": False, | ||
} | ||
self.supercell_gen_kwargs.update(supercell_gen_kwargs if supercell_gen_kwargs is not None else {}) | ||
self.interstitial_gen_kwargs: dict[str, Union[int, float, bool]] = ( | ||
interstitial_gen_kwargs if interstitial_gen_kwargs is not None else {} | ||
) | ||
self.target_frac_coords = target_frac_coords if target_frac_coords is not None else [0.5, 0.5, 0.5] | ||
specified_min_image_distance = self.supercell_gen_kwargs.get("min_image_distance", 10) | ||
specified_min_image_distance = self.supercell_gen_kwargs["min_image_distance"] | ||
|
||
if len(self.structure) == 1 and not self.generate_supercell: | ||
# raise error if only one atom in primitive cell and no supercell generated, as vacancy will | ||
|
@@ -1258,22 +1265,16 @@ class (such as ``clustering_tol``, ``stol``, ``min_dist`` etc), or to | |
with warnings.catch_warnings(): | ||
warnings.filterwarnings("ignore", message="The 'warn' method is deprecated") | ||
supercell_matrix = get_ideal_supercell_matrix( | ||
primitive_structure, | ||
min_atoms=self.supercell_gen_kwargs.get("min_atoms", 50), # different from current | ||
# pymatgen-analysis-defects default `min_atoms` ( = 80) | ||
min_image_distance=specified_min_image_distance, # same as current | ||
# pymatgen-analysis-defects default `min_length` ( = 10) | ||
ideal_threshold=self.supercell_gen_kwargs.get("ideal_threshold", 0.1), | ||
force_cubic=self.supercell_gen_kwargs.get("force_cubic", False), | ||
force_diagonal=self.supercell_gen_kwargs.get("force_diagonal", False), | ||
structure=primitive_structure, | ||
pbar=pbar, | ||
**self.supercell_gen_kwargs, # type: ignore | ||
) | ||
|
||
if not self.generate_supercell or ( | ||
input_min_image_distance >= specified_min_image_distance | ||
and (primitive_structure * supercell_matrix).num_sites | ||
>= self.structure.num_sites | ||
>= self.supercell_gen_kwargs.get("min_atoms", 0) | ||
>= self.supercell_gen_kwargs["min_atoms"] | ||
): | ||
if input_min_image_distance < 10: | ||
# input structure is <10 Å in at least one direction, and generate_supercell=False, | ||
|
@@ -1285,7 +1286,6 @@ class (such as ``clustering_tol``, ``stol``, ``min_dist`` etc), or to | |
f"using input structure as defect & bulk supercells. Caution advised!" | ||
) | ||
|
||
# else input structure is greater than ``min_image_distance`` Å in each direction, and | ||
# ``generate_supercell=False`` or input structure has fewer or same number of atoms as | ||
# doped supercell, so use input structure: | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the documentation to clarify the new behavior of VBM and Fermi level calculation.
The documentation for the
formation_energy
method has been updated to reflect changes in handling the VBM eigenvalue as the reference point for calculating formation energy. Ensure that the new behavior is clearly documented, especially the conditions under which the VBM eigenvalue is used or not used.Ensure consistency in the handling of VBM across different methods.
The handling of the VBM eigenvalue as a reference point for calculations is crucial for accurate defect analysis. It appears in both the
formation_energy
andequilibrium_concentration
methods. Consistency in its usage across these methods is essential for the integrity of the calculations. Consider refactoring to centralize the handling of VBM to ensure consistency and reduce potential errors.Also applies to: 1113-1117