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

Fix PositionD parsing in config from strings like '123.1, 234.2', which had been doing the wrong thing. #1300

Merged
merged 5 commits into from
Jul 10, 2024
Merged
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
18 changes: 7 additions & 11 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ jobs:
brew link --overwrite fftw gcc wget || true
brew install eigen || true
brew link --overwrite eigen || true
brew install --cask gfortran || true
ls $FFTW_DIR

- name: Download des_data
Expand All @@ -141,13 +140,6 @@ jobs:
if ! test -d $HOME/des_data || ! test -f $HOME/des_data/DECam_00154912_01.fits.fz; then wget --no-check-certificate http://www.sas.upenn.edu/~mjarvis/des_data.tar.gz; tar xfz des_data.tar.gz -C $HOME; fi
ln -s $HOME/des_data examples/des/

- name: Install numpy 2.0.0rc2
# Just on python 3.12, use the rc2 branch of numpy 2.0.0.
# XXX: Once the default numpy is >= 2.0, remove this.
if: matrix.py == 3.12
run: |
pip install numpy==2.0.0rc2

- name: Install basic dependencies
run: |
python -m pip install -U pip
Expand All @@ -167,16 +159,20 @@ jobs:
pip install -U matplotlib

- name: Install astroplan
# astroplan isn't yet numpy 2.0 compatible. So don't install that on 3.12.
if: matrix.py != 3.12
# astroplan isn't yet numpy 2.0 compatible. So don't install that on 3.9+.
# I'm also getting errors with starlink, which I think may be related to
# numpy 2.0.
if: (matrix.py == 3.8) || (matrix.py == 3.9)
run: |
pip install -U astroplan
pip install -U starlink-pyast

- name: Install CPython-only dependencies
if: matrix.py != 'pypy-3.7'
run: |
# The only one left that doesn't seem to work right on pypy is starlink-pyast.
pip install -U starlink-pyast
# (This is now above in numpy 1.x versions.)
#pip install -U starlink-pyast
# And now pandas is acting up on pypy.
# cf. https://github.com/pandas-dev/pandas/issues/44253
pip install -U pandas
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,8 @@ Changes from v2.5.2 to v2.5.3
- Added ScaleFlux and ScaleWavelength photon ops. (#1289)
- Deprecated ChromaticObject.atRedshift. (#1291)
- Various fixes to work with numpy 2.0. (#1297)

Changes from v2.5.3 to v2.5.4
=============================

- Fixed a bug in the config layer parsing of PositionD from a string. (#1299)
5 changes: 3 additions & 2 deletions galsim/config/value.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,9 @@ def _GetPositionValue(param):
"""Convert a tuple or a string that looks like "a,b" into a galsim.PositionD.
"""
try:
x = float(param[0])
y = float(param[1])
x, y = param
x = float(x)
y = float(y)
except (ValueError, TypeError):
try:
x, y = param.split(',')
Expand Down
6 changes: 6 additions & 0 deletions tests/test_config_value.py
Original file line number Diff line number Diff line change
Expand Up @@ -1432,6 +1432,7 @@ def test_pos_value():
'val1' : galsim.PositionD(0.1,0.2),
'val2' : '0.1, 0.2',
'val3' : None,
'val4' : '123.4, 567.8',
'xy1' : { 'type' : 'XY', 'x' : 1.3, 'y' : 2.4 },
'ran1' : { 'type' : 'RandomCircle', 'radius' : 3 },
'ran2' : { 'type' : 'RandomCircle', 'radius' : 1, 'center' : galsim.PositionD(3,7) },
Expand Down Expand Up @@ -1487,6 +1488,10 @@ def test_pos_value():
val3 = galsim.config.ParseValue(config,'val3',config, galsim.PositionD)[0]
np.testing.assert_equal(val3, None)

val4 = galsim.config.ParseValue(config,'val4',config, galsim.PositionD)[0]
np.testing.assert_almost_equal(val4.x, 123.4)
np.testing.assert_almost_equal(val4.y, 567.8)

xy1 = galsim.config.ParseValue(config,'xy1',config, galsim.PositionD)[0]
np.testing.assert_almost_equal(xy1.x, 1.3)
np.testing.assert_almost_equal(xy1.y, 2.4)
Expand Down Expand Up @@ -1594,6 +1599,7 @@ def test_pos_value():
# Finally, these value got changed, so they won't match the original
# unless we manually set them back to the original strings.
clean_config['val2'] = '0.1, 0.2'
clean_config['val4'] = '123.4, 567.8'
clean_config['cur2'] = '@input.val2'
clean_config['input']['val2'] = '0.3, 0.4'
assert clean_config == orig_config
Expand Down
4 changes: 2 additions & 2 deletions tests/test_zernike.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ def Z11_grad(x, y):
# fig.colorbar(scat2, ax=axes[2])
# plt.show()

np.testing.assert_allclose(Z11.evalCartesianGrad(x, y), Z11_grad(x, y), rtol=0, atol=1e-12)
np.testing.assert_allclose(Z11.evalCartesianGrad(x, y), Z11_grad(x, y), rtol=1.e-12, atol=1e-12)

Z28 = Zernike([0]*28+[1])

Expand All @@ -400,7 +400,7 @@ def Z28_grad(x, y):
grady = -6*np.sqrt(14)*y*(5*x**4 - 10*x**2*y**2 + y**4)
return gradx, grady

np.testing.assert_allclose(Z28.evalCartesianGrad(x, y), Z28_grad(x, y), rtol=0, atol=1e-12)
np.testing.assert_allclose(Z28.evalCartesianGrad(x, y), Z28_grad(x, y), rtol=1.e-12, atol=1e-12)

# Now try some finite differences on a broader set of input

Expand Down
Loading