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

1.1.1: Testing astropy.wcs fails with Python-3.5 and wcslib-5.12 #4460

Closed
olebole opened this issue Jan 9, 2016 · 17 comments
Closed

1.1.1: Testing astropy.wcs fails with Python-3.5 and wcslib-5.12 #4460

olebole opened this issue Jan 9, 2016 · 17 comments
Assignees

Comments

@olebole
Copy link
Member

olebole commented Jan 9, 2016

On Debian, the astropy.wcs tests fails reproducible with some corrupted memory:

astropy/wcs/tests/test_wcs.py::test_calc_footprint_3 PASSED
astropy/wcs/tests/test_wcs.py::test_sip PASSED
astropy/wcs/tests/test_wcs.py::test_printwcs PASSED
astropy/wcs/tests/test_wcs.py::test_invalid_spherical PASSED
astropy/wcs/tests/test_wcs.py::test_no_iteration PASSED
astropy/wcs/tests/test_wcs.py::test_sip_tpv_agreement PASSED
astropy/wcs/tests/test_wcs.py::test_tpv_copy PASSED
astropy/wcs/tests/test_wcs.py::test_hst_wcs *** Error in `/usr/bin/python3.5':
    free(): corrupted unsorted chunks: 0x000000000bc2d530 ***

I wonder whether this is connected with #4441. wcslib is compiled here with flex-2.5.39 (the current version in Debian).

Version 1.1 compiles without this problem, so this is a regression.

@embray
Copy link
Member

embray commented Jan 13, 2016

Probably related to #4441. I doubt this is particular to v1.1.1, which did not contain any significant changes to WCS code at the C level. That is, it probably is not a regression compared to v1.1, just to be clear.

@lupinix
Copy link

lupinix commented Jan 13, 2016

I can confirm the issue with Fedora. We are working on wcslib update there. Builds with wcslib 4.25.1 don't show this issue.

astropy/wcs/tests/test_pickle.py ......
astropy/wcs/tests/test_profiling.py ..................................
astropy/wcs/tests/test_utils.py .........................................
astropy/wcs/tests/test_wcs.py ......................................................................./var/tmp/rpm-tmp.7zgjNg: line 36: 28927 Aborted                 (core dumped) py.test-3.5 -k "not test_web_profile" astropy
error: Bad exit status from /var/tmp/rpm-tmp.7zgjNg (%check)

Full build log at https://copr-be.cloud.fedoraproject.org/results/lupinix/wcslib5-test/fedora-rawhide-x86_64/00153269-python-astropy/build.log.gz

@olebole
Copy link
Member Author

olebole commented Jan 14, 2016

@embray I tested both versions 1.1 and 1.1.1 in an identical environment (including wcslib 5.12) astropy-1.1 does not have this issue, but astropy-1.1.1 has.

@olebole
Copy link
Member Author

olebole commented Jan 14, 2016

I could track this down to commit 6d4552e: After reverting this patch, all tests succeed (on x86_64). I agree that this is strange; however I doublechecked it.

@embray
Copy link
Member

embray commented Jan 14, 2016

How strange. Have you managed to get a stack trace for the error? I almost wonder if something is awry with the re module in Python 3.5.

@embray
Copy link
Member

embray commented Jan 14, 2016

Though I'll also need to go through that commit with a fine-toothed comb and make sure it's not doing anything materially different from the previous code. It looked right to me before but maybe there's a subtle mistake in it somewhere.

@astrofrog
Copy link
Member

@olebole - could you also try and simplify the regular expression to see if the error goes away?

@embray
Copy link
Member

embray commented Jan 14, 2016

Yeah looking at it again I see several areas where it could be simplified (not that it's that complex in the first place).

@embray
Copy link
Member

embray commented Jan 14, 2016

Here's a simpler equivalent regexp:

'''^[AB]P?_1?[0-9]_1?[0-9][A-Z]?$'''

@embray
Copy link
Member

embray commented Jan 14, 2016

See also this note: 6d4552e#commitcomment-15453711

That might also be related--I'm not sure if it's properly deleting all the SIP keywords because of this.

@embray
Copy link
Member

embray commented Jan 14, 2016

@olebole @lupinix Could one or both of you confirm whether the patch in #4492 fixes the issue? Thanks.

@embray embray self-assigned this Jan 14, 2016
@olebole
Copy link
Member Author

olebole commented Jan 14, 2016

I just made a test build with both the simplified regexp and the list() wrapping, which was successful; they fix the issue. Do you need to have them tested separately as well?

@embray
Copy link
Member

embray commented Jan 14, 2016

@olebole I'd be curious, yeah. At this point I think the fix in #4492 is more likely the culprit than anything in the regexp (though I'll probably commit the simplified regexp separately too).

@lupinix
Copy link

lupinix commented Jan 14, 2016

I can confirm: Builds fine now: https://copr.fedoraproject.org/coprs/lupinix/wcslib5-test/build/153475/

Xarthisius added a commit to Xarthisius/astropy that referenced this issue Jan 14, 2016
Regex optimization by Erik Bray <erik.m.bray@gmail.com>
See also: issue astropy#4460 (comments)

Signed-off: Kacper Kowalik <xarthisius.kk@gmail.com>
@olebole
Copy link
Member Author

olebole commented Jan 14, 2016

I can confirm that the list() wrapping in #4492 solved the problem.

embray pushed a commit that referenced this issue Jan 19, 2016
Regex optimization by Erik Bray <erik.m.bray@gmail.com>
See also: issue #4460 (comments)

Signed-off: Kacper Kowalik <xarthisius.kk@gmail.com>
@astrofrog
Copy link
Member

@olebole - so just to be clear, we can close this?

@olebole
Copy link
Member Author

olebole commented Feb 9, 2016

Yep.

@olebole olebole closed this as completed Feb 9, 2016
dhomeier pushed a commit to dhomeier/astropy that referenced this issue Jun 12, 2016
Regex optimization by Erik Bray <erik.m.bray@gmail.com>
See also: issue astropy#4460 (comments)

Signed-off: Kacper Kowalik <xarthisius.kk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants