Skip to content

Commit

Permalink
Flatten q-point indices for compatibility with Numpy 2.0 (#303)
Browse files Browse the repository at this point in the history
* Flatten q-point indices for compatibility with Numpy 2.0

With Numpy 2.0, np.unique returns a 2-D array when applied to
q-points. This in turn gets broadcast and adds an extra dimension to
the frequency/mode arrays, causing all kinds of chaos.

Flattening the array here should be a no-op for Numpy <2 and fixes the
issue.

*  Update deprecated scipy.integrate simps -> simpson

*  Pin Mac tests to macos-12 environment for Brille compatibility

Hopefully we can return to macos-latest soon once Brille build issues are resolved
  • Loading branch information
ajjackson authored Jul 9, 2024
1 parent be51d81 commit 870a9ef
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build_upload_pypi_wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
include:
- os: windows-latest
wheelname: win
- os: macos-latest
- os: macos-12
wheelname: macos
- os: ubuntu-latest
wheelname: manylinux
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/run_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
(github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'no_ci'))
strategy:
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
os: [ubuntu-latest, windows-latest, macos-12]
fail-fast: false
runs-on: ${{ matrix.os }}
steps:
Expand All @@ -29,7 +29,8 @@ jobs:
channel-priority: true
- name: Install llvm on Macos
if: startsWith(matrix.os, 'macos')
run: brew install llvm
run: |
brew install llvm
- name: Update pip and install dependencies
shell: bash -l {0}
run: |
Expand Down Expand Up @@ -96,7 +97,7 @@ jobs:
if: success() || failure()
steps:
- name: Download Artifacts
uses: actions/download-artifact@v3
uses: actions/download-artifact@v4
with:
path: artifacts
- name: Publish test results
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/test_release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
test:
strategy:
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
os: [ubuntu-latest, windows-latest, macos-12]
fail-fast: false
runs-on: ${{ matrix.os }}
steps:
Expand All @@ -22,7 +22,8 @@ jobs:
channel-priority: true
- name: Install llvm on Macos
if: startsWith(matrix.os, 'macos')
run: brew install llvm
run: |
brew install llvm
- name: Update pip and install dependencies
shell: bash -l {0}
run: |
Expand Down
11 changes: 11 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@
spglib raises TypeError when using empty unit cell and this needs
handling when looking for high-symmetry labels

- Compatibility fix for Numpy 2.0 update: avoid some
broadcasting issues with array shape returned by ``np.unique``

- Update reference to scipy.integrate.simpson (scipy.integrate.simps
is deprecated)

- Mac tests and builds are running against "macos-12" github runner
instead of "macos-latest", in order to test properly against
Brille. This should be restored to "macos-latest" when Brille
build system is updated and gives consistent results with
win/linux.

-------------------------------------------------------------------------------

Expand Down
2 changes: 2 additions & 0 deletions euphonic/force_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ def _calculate_phonons_at_qpts(
norm_qpts[gamma_i] = 0.
reduced_qpts, qpts_i = np.unique(norm_qpts, return_inverse=True,
axis=0)
qpts_i = qpts_i.flatten()

n_rqpts = len(reduced_qpts)
# Special handling of gamma points - don't reduce gamma
# points if LO-TO splitting
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def get_c_extension():
else:
link_args = ['-fopenmp']

compile_args = ['-fopenmp']
compile_args = ['-fopenmp']

else:
# Linux - assume gcc if CC not set
Expand Down
8 changes: 4 additions & 4 deletions tests_and_analysis/test/euphonic_test/test_broadening.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from numpy.polynomial import Polynomial
from numpy.random import RandomState
import numpy.testing as npt
from scipy.integrate import simps
from scipy.integrate import simpson
from scipy.ndimage import gaussian_filter

from euphonic.broadening import (find_coeffs,
Expand Down Expand Up @@ -74,10 +74,10 @@ def test_area_unchanged_for_broadened_dos(material, qpt_freqs_json,
fit='cheby-log')
ebins_centres = ebins.magnitude[:-1] + 0.5*np.diff(ebins.magnitude)
assert dos.y_data.units == 1/ebins.units
dos_area = simps(dos.y_data.magnitude, ebins_centres)
dos_area = simpson(dos.y_data.magnitude, x=ebins_centres)
assert variable_width_broaden.units == 1/ebins.units
adaptively_broadened_dos_area = simps(variable_width_broaden.magnitude,
ebins_centres)
adaptively_broadened_dos_area = simpson(
variable_width_broaden.magnitude, x=ebins_centres)
assert adaptively_broadened_dos_area == pytest.approx(dos_area, rel=0.01)


Expand Down

0 comments on commit 870a9ef

Please sign in to comment.