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

Sphinx log parser fix #1556

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ apidoc/
.ipynb_checkpoints/
test_times.dat
core.*
.vscode/settings.json
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.vscode/settings.json

Or does this have a particular reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use vscode and it usually pushes changes in my settings to git sometimes

Copy link
Member

Choose a reason for hiding this comment

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

I think it is very helpful for all vscode users, just like including the .idea/ folder in the .gitignore is helpful for Pycharm users. So we could debate if this should be a separate pull request, but in general I think it is a helpful addition.

24 changes: 22 additions & 2 deletions pyiron_atomistics/sphinx/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,7 @@ def load_default_groups(self):
if self._spin_enabled:
self.input["EmptyStates"] = int(1.5 * len(self.structure) + 3)
else:
self.input["EmptyStates"] = int(len(self.structure) + 3)
self.input["EmptyStates"] = int(0.5 * len(self.structure) + 3)

if not self.input.sphinx.basis.read_only:
self.load_basis_group()
Expand Down Expand Up @@ -1486,7 +1486,22 @@ def convergence_check(self):
"""
# Checks if sufficient empty states are present
if not self.nbands_convergence_check():
print(
"""
The highest orbital is not empty (has a finite occupation).
Number of empty states might be too few.
skatnagallu marked this conversation as resolved.
Show resolved Hide resolved
Try changing to a higher number of empty states or use default values.
"""
)
return False
if not self.output.generic.dft.scf_convergence[-1]:
print(
"""
scf convergence not reached. Check residue and number of steps.
If residue goes down systematically, but slowly, try increasing number of steps.
If residue is high, decreasing rho mixing might help.
"""
)
return self.output.generic.dft.scf_convergence[-1]

def collect_logfiles(self):
Expand Down Expand Up @@ -1592,7 +1607,12 @@ def check_setup(self):
else:
warnings.warn(
"Number of empty states was not specified. Default: "
+ "3+NIONS for non-magnetic systems"
+ "3+NIONS*0.5 for non-magnetic systems"
)
else:
if self.input["EmptyStates"] < 0.5 * int(len(self.structure) + 3):
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this normal for semi conductors? @skatnagallu @freyso

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For semiconductors this doesnt matter right? You shouldn't set it for semiconductors. And then SPHInX defaults to 0.

warnings.warn(
"Number of empty states seem to be too low. Hopefully you know what you are doing."
)

return len(w) == 0
Expand Down
4 changes: 2 additions & 2 deletions pyiron_atomistics/sphinx/output_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ def index_permutation(self):

@property
def spin_enabled(self):
return len(re.findall("The spin for the label", self.log_file)) > 0
return len(re.findall("Spin moment:", self.log_file)) > 0

@property
def log_main(self):
Expand Down Expand Up @@ -405,7 +405,7 @@ def get_convergence(self):
return convergence

def get_fermi(self):
pattern = r"Fermi energy:\s+(\d+\.\d+)\s+eV"
pattern = r"Fermi energy:\s+(-?\d+\.\d+)\s+eV"
return np.array(re.findall(pattern, self.log_main)).astype(float)

@property
Expand Down
Loading