Skip to content

Commit

Permalink
Merge branch 'main' into jwa7-patch-1
Browse files Browse the repository at this point in the history
  • Loading branch information
jwa7 committed Jan 31, 2025
2 parents 12eced8 + 5b04644 commit 7a4c85c
Show file tree
Hide file tree
Showing 13 changed files with 68 additions and 13 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build-wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ jobs:
- name: Comment with download link
uses: PicoCentauri/comment-artifact@v1
if: github.event.pull_request.head.repo.fork == false
with:
name: wheels
description: ⚙️ [Download Python wheels for this pull-request (you can install these with pip)
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ jobs:

- name: Comment with download link
uses: PicoCentauri/comment-artifact@v1
if: github.event.pull_request.head.repo.fork == false
with:
name: docs
description: 📚 Download documentation for this pull-request
Expand Down
2 changes: 1 addition & 1 deletion featomic/src/c_api/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pub struct featomic_system_t {
unsafe impl Send for featomic_system_t {}
unsafe impl Sync for featomic_system_t {}

impl<'a> System for &'a mut featomic_system_t {
impl System for &mut featomic_system_t {
fn size(&self) -> Result<usize, Error> {
let function = self.size.ok_or_else(|| Error::External {
status: FEATOMIC_SYSTEM_ERROR,
Expand Down
2 changes: 1 addition & 1 deletion featomic/src/calculator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ fn map_selection_error<'a>(
};
}

impl<'a> LabelsSelection<'a> {
impl LabelsSelection<'_> {
fn select<'call, F, G, H>(
&self,
label_kind: &str,
Expand Down
4 changes: 2 additions & 2 deletions featomic/src/calculators/shared/descriptors_by_systems.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@ pub struct TensorMapView<'a> {
marker: std::marker::PhantomData<&'a mut TensorMap>,
}

impl<'a> std::ops::Deref for TensorMapView<'a> {
impl std::ops::Deref for TensorMapView<'_> {
type Target = TensorMap;

fn deref(&self) -> &Self::Target {
&self.data
}
}

impl<'a> std::ops::DerefMut for TensorMapView<'a> {
impl std::ops::DerefMut for TensorMapView<'_> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.data
}
Expand Down
2 changes: 1 addition & 1 deletion featomic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

#![allow(clippy::cast_possible_truncation, clippy::cast_precision_loss)]
#![allow(clippy::cast_possible_wrap, clippy::cast_lossless, clippy::cast_sign_loss)]
#![allow(clippy::default_trait_access)]
#![allow(clippy::default_trait_access, clippy::empty_line_after_doc_comments)]

// Tests lints
#![cfg_attr(test, allow(clippy::float_cmp))]
Expand Down
2 changes: 1 addition & 1 deletion featomic/src/systems/chemfiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::systems::UnitCell;

use super::{System, SimpleSystem};

impl<'a> From<&'a chemfiles::Frame> for Box<dyn System> {
impl From<&chemfiles::Frame> for Box<dyn System> {
fn from(frame: &chemfiles::Frame) -> Self {
let mut assigned_types = HashMap::new();
let mut get_atomic_type = |atom: chemfiles::AtomRef| {
Expand Down
4 changes: 2 additions & 2 deletions featomic/src/types/vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,15 +275,15 @@ impl Neg for Vector3D {
}
}

impl<'a> Neg for &'a Vector3D {
impl Neg for &Vector3D {
type Output = Vector3D;
#[inline]
fn neg(self) -> Vector3D {
Vector3D::new(-self[0], -self[1], -self[2])
}
}

impl<'a> Neg for &'a mut Vector3D {
impl Neg for &mut Vector3D {
type Output = Vector3D;
#[inline]
fn neg(self) -> Vector3D {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ def compute(
self,
systems: Union[IntoSystem, List[IntoSystem]],
selected_keys: Optional[Labels] = None,
selected_samples: Optional[Labels] = None,
neighbors_to_properties: bool = False,
) -> TensorMap:
"""
Expand Down Expand Up @@ -245,6 +246,11 @@ def compute(
:param selected_keys: :py:class:`Labels`, the output keys to computed. If
``None``, all keys are computed. Subsets of key dimensions can be passed to
compute output blocks that match in these dimensions.
:param selected_samples: :py:class:`Labels`, Set of samples on which to run the
calculation. Use ``None`` to run the calculation on all samples in
the systems (this is the default). Gets passed to ``calculator_1`` and
``calculator_2``, therefore requiring that both calculators support sample
selection.
:param neighbors_to_properties: :py:class:`bool`, if true, densifies the
spherical expansion by moving key dimension "neighbor_type" to properties
prior to performing the Clebsch Gordan product step. Defaults to false.
Expand All @@ -254,6 +260,7 @@ def compute(
return self._equivariant_power_spectrum(
systems=systems,
selected_keys=selected_keys,
selected_samples=selected_samples,
neighbors_to_properties=neighbors_to_properties,
compute_metadata=False,
)
Expand All @@ -262,6 +269,7 @@ def forward(
self,
systems: Union[IntoSystem, List[IntoSystem]],
selected_keys: Optional[Labels] = None,
selected_samples: Optional[Labels] = None,
neighbors_to_properties: bool = False,
) -> TensorMap:
"""
Expand All @@ -275,13 +283,15 @@ def forward(
return self.compute(
systems=systems,
selected_keys=selected_keys,
selected_samples=selected_samples,
neighbors_to_properties=neighbors_to_properties,
)

def compute_metadata(
self,
systems: Union[IntoSystem, List[IntoSystem]],
selected_keys: Optional[Labels] = None,
selected_samples: Optional[Labels] = None,
neighbors_to_properties: bool = False,
) -> TensorMap:
"""
Expand All @@ -294,6 +304,7 @@ def compute_metadata(
return self._equivariant_power_spectrum(
systems=systems,
selected_keys=selected_keys,
selected_samples=selected_samples,
neighbors_to_properties=neighbors_to_properties,
compute_metadata=True,
)
Expand All @@ -302,19 +313,24 @@ def _equivariant_power_spectrum(
self,
systems: Union[IntoSystem, List[IntoSystem]],
selected_keys: Optional[Labels],
selected_samples: Optional[Labels],
neighbors_to_properties: bool,
compute_metadata: bool,
) -> TensorMap:
"""
Computes the equivariant power spectrum, either fully or just metadata
"""
# Compute density
density_1 = self.calculator_1.compute(systems)
density_1 = self.calculator_1.compute(
systems, selected_samples=selected_samples
)

if self.calculator_2 is None:
density_2 = density_1
else:
density_2 = self.calculator_2.compute(systems)
density_2 = self.calculator_2.compute(
systems, selected_samples=selected_samples
)

# Rename "neighbor_type" dimension so they are correlated
density_1 = operations.rename_dimension(
Expand Down
37 changes: 37 additions & 0 deletions python/featomic/tests/clebsch_gordan/equivariant_power_spectrum.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,43 @@ def test_equivariant_power_spectrum_neighbors_to_properties():
metatensor.equal_raise(powspec_1, powspec_2)


def test_sample_selection() -> None:
"""Tests that the sample selection works as expected.
By first computing the powerspectruim for all atoms in H2O
Then first for atom 1 and then atom 2 and 3.
Their join should be identical to computing it for all atoms.
"""

frame = h2o_periodic()

powspec_calc = EquivariantPowerSpectrum(SphericalExpansion(**SPHEX_HYPERS_SMALL))

label_1st = metatensor.Labels(
["system", "atom"], np.array([[0, 0]], dtype=np.int32)
)

label_2nd = metatensor.Labels(
["system", "atom"], np.array([[0, 1], [0, 2]], dtype=np.int32)
)

powspec_1 = powspec_calc.compute(
frame, neighbors_to_properties=True, selected_samples=label_1st
)

powspec_2 = powspec_calc.compute(
frame, neighbors_to_properties=True, selected_samples=label_2nd
)

powspec_3 = metatensor.join(
[powspec_1, powspec_2], axis="samples", remove_tensor_name=True
)
powspec_4 = powspec_calc.compute(frame, neighbors_to_properties=True)

assert metatensor.equal(powspec_3, powspec_4)
assert not metatensor.equal(powspec_2, powspec_4)
assert not metatensor.equal(powspec_1, powspec_4)


def test_fill_types_option() -> None:
"""
Test that ``neighbor_types`` options adds arbitrary atomic neighbor types.
Expand Down
2 changes: 1 addition & 1 deletion python/featomic/tests/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def record_log_events(level, message):
calculator.compute(SystemForTests())

message = (
"featomic::calculators::dummy_calculator -- " "log-test-info: test info message"
"featomic::calculators::dummy_calculator -- log-test-info: test info message"
)
event = (FEATOMIC_LOG_LEVEL_INFO, message)
assert event in recorded_events
Expand Down
2 changes: 1 addition & 1 deletion python/featomic/tests/systems/ase.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def test_partial_pbc():
atoms.pbc = [True, True, False]

message = (
"different periodic boundary conditions on different axis " "are not supported"
"different periodic boundary conditions on different axis are not supported"
)
with pytest.raises(ValueError, match=message):
AseSystem(atoms)
Expand Down
2 changes: 1 addition & 1 deletion python/scripts/generate-declarations.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def funcdecl_to_ctypes(type, ndpointer=False):
restype = type_to_ctypes(type.type, ndpointer)
args = [type_to_ctypes(t.type, ndpointer) for t in type.args.params]

return f'CFUNCTYPE({restype}, {", ".join(args)})'
return f"CFUNCTYPE({restype}, {', '.join(args)})"


def type_to_ctypes(type, ndpointer=False):
Expand Down

0 comments on commit 7a4c85c

Please sign in to comment.