Skip to content

Commit

Permalink
Fix regression in Gamp writing
Browse files Browse the repository at this point in the history
There was an issue with GAMP not writing data again that's a regression
from the patches for v3.2.0. If I had to guess, the patches to avoid
deprecations from Numpy changed the internal data types for the vectors
in a minor way that caused the writer to break. In v3.2.0 it's assumed
the internal values when extracted by a single value would be a scalar,
the patch to 3.2.1 assumes it's an array with a single element, and then
one of the changes in 3.2.2 changed that internal value back to a
scalar.

This patch doesn't revert to assuming the data is a scalar, instead we
now wrap the writing data with `float` so that regardless of whether
it's a scalar or an array with a single element. Hopefully that means
this particular issue will no longer present itself.

As a bonus, ParticlePools can now be compared against each other. You
can now `p1_pool == p2_pool` and recieve a boolean result depending on
if they are equal or not. While there is not a lot of praticle use for
this for Physics outside of sanity checking, it does make the process of
unit testing the libraries much more manageable.
  • Loading branch information
markjonestx committed Jun 11, 2021
1 parent e74253c commit a6ed803
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 18 deletions.
13 changes: 12 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@ All changes important to the user will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/)

## [3.2.3] - 2021-6-11
### Added
- Particle Pools can now compared against other Particle Pools to see if they
are storing the same content.
### Fixed
- Regression from 3.2.0 where Gamp would not write out data to disk. This time
by wrapping the data in a float, which should catch instances where the value
stored is a pure scalar, verses instances where the data is an array with a
len == 1

## [3.2.2] - 2021-6-11
### Fixed
- Particles can now be masked again, the mask is no longer silently deleted
Expand Down Expand Up @@ -202,7 +212,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/)
- PySim plugin
- Packaging

[Unreleased]: https://github.com/JeffersonLab/PyPWA/compare/v3.2.2...main
[Unreleased]: https://github.com/JeffersonLab/PyPWA/compare/v3.2.3...main
[3.2.3]: https://github.com/JeffersonLab/PyPWA/compare/v3.2.2...v.3.2.3
[3.2.2]: https://github.com/JeffersonLab/PyPWA/compare/v3.2.1...3.2.2
[3.2.1]: https://github.com/JeffersonLab/PyPWA/compare/v3.2.0...v3.2.1
[3.2.0]: https://github.com/JeffersonLab/PyPWA/compare/v3.1.0...v3.2.0
Expand Down
2 changes: 1 addition & 1 deletion PyPWA/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
__credits__ = ["Mark Jones"]

AUTHOR = "PyPWA Team and Contributors"
VERSION = "3.2.2"
VERSION = "3.2.3"
RELEASE = f"{VERSION}"
LICENSE = "GPLv3"
STATUS = "development"
21 changes: 8 additions & 13 deletions PyPWA/libs/vectors/four_vector.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,15 @@ def __eq__(self, vector: "FourVector") -> bool:
return self._compare_vectors(vector)

def _compare_vectors(self, other):
if all([hasattr(other, attr)] for attr in self.__slots__):
equality = []
for slot in self.__slots__:
result = np.equal(getattr(self, slot), getattr(other, slot))

if isinstance(self._x, np.ndarray):
equality.append(all(result))
else:
equality.append(result)

return all(equality)

else:
if not all(other.e == self.e):
return False
if not all(other.x == self.x):
return False
if not all(other.y == self.y):
return False
if not all(other.z == self.z):
return False
return True

def __truediv__(self, scalar: Union[float, int]) -> "FourVector":
if isinstance(scalar, (int, float)):
Expand Down
19 changes: 18 additions & 1 deletion PyPWA/libs/vectors/particle.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ class Particle(FourVector):
ParticlePool : For storing a collection of particles
"""

__slots__ = ["_vector", "__particle_id", "__particle_name", "__charge"]
__slots__ = [
"_e", "_x", "_y", "_z", "__particle_id", "__particle_name", "__charge"
]

def __init__(
self,
Expand Down Expand Up @@ -332,6 +334,21 @@ def __getitem__(self, item):
return self._mask(item)
return self.__particle_list[item]

def __eq__(self, other):
if not isinstance(other, ParticlePool):
return False
if other.event_count != self.event_count:
return False
if other.particle_count != self.particle_count:
return False

particle_pair = zip(self.iter_particles(), other.iter_particles())
for current_particle, other_particle in particle_pair:
if current_particle != other_particle:
return False

return True

def _mask(self, mask):
print("PP Masking")
if not len(mask) == len(self.__particle_list[0]):
Expand Down
3 changes: 2 additions & 1 deletion PyPWA/plugins/data/gamp.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ def write(self, data: vectors.ParticlePool):
for p in data.iter_particles():
self.__file_handle.write(
"%d %d %.20f %.20f %.20f %.20f\n" % (
p.id, p.charge, p.x[0], p.y[0], p.z[0], p.e[0]
p.id, p.charge,
float(p.x), float(p.y), float(p.z), float(p.e)
)
)

Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

__author__ = "PyPWA Team and Contributors"
__license__ = "GPLv3"
__version__ = "3.2.2"
__version__ = "3.2.3"
__email__ = "[email protected]"
__status__ = "development"

Expand Down
9 changes: 9 additions & 0 deletions tests/plugins/data/test_gamp.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from PyPWA.plugins.data import gamp

ROOT = (Path(__file__).parent / "../../test_data/docs").resolve()
TEMP_LOCATION = ROOT / "temporary_write_data.gamp"
LARGE = ROOT / "large.gamp"
MULTI = ROOT / "multiple.gamp"

Expand Down Expand Up @@ -38,3 +39,11 @@ def test_large_has_four_particles(large_gamp):

def test_multi_has_six_particles(multi_gamp):
assert 6 == len(multi_gamp)


def test_write_data(multi_gamp, gamp_mem):
gamp_mem.write(TEMP_LOCATION, multi_gamp)
intermediate = gamp_mem.parse(TEMP_LOCATION)
TEMP_LOCATION.unlink(True) # clear the space after it's loaded.

assert intermediate == multi_gamp

0 comments on commit a6ed803

Please sign in to comment.