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.

Sign up for GitHub

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

SOF-7417: consider non-collinear case while parsing qe bandstructure #146

Merged
merged 9 commits into from
Aug 14, 2024
Merged

SOF-7417: consider non-collinear case while parsing qe bandstructure #146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
189ab2b
SOF-7417: consider non-collinear case while parsing qe bandstructure
pranabdas Aug 3, 2024
3dec51f
pin numpy version to v1 for now
pranabdas Aug 3, 2024
9c2356a
pin numpy version to v1.24.4 to support python 3.8
pranabdas Aug 3, 2024
001bdb9
SOF-7417: fix non-colin nspins for bandstructure plot
pranabdas Aug 5, 2024
2304b3c
SOF-7417: chore ==> run github workflows on ubuntu-22.04
pranabdas Aug 5, 2024
ac25885
SOF-7417: chore ==> rewrite the ternary condition
pranabdas Aug 6, 2024
7eeb828
SOF-7417: set numpy>=1.24.4,<2
pranabdas Aug 7, 2024
5cdfc20
SOF-7417: chore ==> spellings
pranabdas Aug 7, 2024
09c8294
SOF-7417: refactor obsolete numpy ndarray.ptp() method
pranabdas Aug 8, 2024
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
4 changes: 2 additions & 2 deletions .github/workflows/cicd.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ concurrency:

jobs:
run-linter:
runs-on: ubuntu-20.04
runs-on: ubuntu-22.04
strategy:
matrix:
python-version:
Expand Down Expand Up @@ -36,7 +36,7 @@ jobs:
python-version: ${{ matrix.python-version }}

run-tests:
runs-on: ubuntu-20.04
runs-on: ubuntu-22.04
strategy:
matrix:
python-version:
Expand Down
6 changes: 1 addition & 5 deletions express/parsers/apps/espresso/formats/xml/xml_post64.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ def eigenvalues_at_kpoints(self) -> list:
all_kpoints = []
bs_tag = self.root.find(self.band_structure_tag)
is_lsda = self._get_xml_tag_value(bs_tag.find("lsda"))
is_noncolinear = self._get_xml_tag_value(bs_tag.find("noncolin"))

if is_lsda:
nband = int(bs_tag.find("nbnd_up").text)
Expand All @@ -144,10 +143,7 @@ def eigenvalues_at_kpoints(self) -> list:
}
if is_lsda:
kpoint_dict["eigenvalues"] = self.__process_ks_lsda(ks_entry, nband)

# TODO: implement noncolinear spin magnetization case, values come in pairs
elif is_noncolinear:
raise NotImplementedError("Noncolinear spin magnetization case not implemented")
# below clause is applicable to both non-magnetic and non-collinear magnetic cases
else:
kpoint_dict["eigenvalues"] = self.__process_ks_non_mag(ks_entry)

Expand Down
2 changes: 1 addition & 1 deletion express/properties/non_scalar/bandgaps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def _find_gap(
Returns:
tuple: a (gap, k1, k2) tuple where k1 and k2 are the indices of the valence and conduction k-points.
"""
if occupations.ptp() > 0:
if np.ptp(occupations) > 0:
# Some band must be crossing fermi-level. Hence, we return zero for band gap and the actual k-points
kv = kc = occupations.argmax()
return 0.0, kv, kc
Expand Down
11 changes: 10 additions & 1 deletion express/properties/non_scalar/two_dimensional_plot/band_structure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,16 @@ class BandStructure(TwoDimensionalPlotProperty):

def __init__(self, name, parser, *args, **kwargs):
super(BandStructure, self).__init__(name, parser, *args, **kwargs)
self.nspins = self.parser.nspins()
# There could be three possibilities for nspins:
# (1) non-magnetic ==> nspins = 1
# (2) collinear magnetic (LSDA) ==> nspins = 2
# (3) non-collinear magnetic ==> nspins = 4
# Prior to Quantum ESPRESSO version 6.4, the XML output contains the tag
# NUMBER_OF_SPIN_COMPONENTS, which is set to 4 for non-collinear case,
# and we return the same for versions above 6.4 if noncolin is True.
# However, for non-collinear magnetic case there is one eigenvalue per
# k-point. Here we override nspins for non-collinear case.
self.nspins = 1 if self.parser.nspins() == 4 else self.parser.nspins()

self.eigenvalues_at_kpoints = self.parser.eigenvalues_at_kpoints()
if kwargs.get("remove_non_zero_weight_kpoints", False):
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ dependencies = [
# "rdkit-pypi>=2022.3.5",
"jarvis-tools>=2023.12.12",
# To avoid module 'numpy.linalg._umath_linalg' has no attribute '_ilp64' in Colab
Copy link
Member Author

Choose a reason for hiding this comment

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

@timurbazhirov what is the numpy version requirement for Colab? There are some breaking (minor) changes on numpy v2 (recently released). For now, we may pin to numpy v1. Also, numpy now requires Python v3.9 or above. Tests are failing on v3.8, because it cannot install numpy v1.26.0. If Colab compatibility allows, we may pin to v1.24.4.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe let's just use "numpy<2" and let pip figure it out?

Copy link
Member

Choose a reason for hiding this comment

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

Including @knc6 - any insight regarding the numpy version for Colab?

Copy link
Member Author

Choose a reason for hiding this comment

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

The root of the problem appears to be presence of multiple numpy versions in the system numpy/numpy#25150 Colab comes with preinstalled set of python packages, including numpy. As of now, Colab uses the latest release of numpy v1. So, with current range specification, we are good for now. But the problem might come back when Colab upgrades to numpy v2 and we are not. We can fix our code in a separate PR, so that we can use numpy v2 as well.

# "numpy==1.23.5",
"numpy>=1.24.4,<2",
]

[project.optional-dependencies]
Expand Down
Loading