Repository navigation
4.0 rc1: Not enough warnings on i386 in test_data_out_of_range #9694
Description
When running on i386, I get the following failures due to missing warnings:
_______________ test_data_out_of_range[False-fast_reader2-True] ________________
parallel = False, fast_reader = {'parallel': False, 'use_fast_converter': True}
guess = True
@pytest.mark.parametrize("guess", [True, False])
# fast_reader configurations: False| 'use_fast_converter'=False|True
@pytest.mark.parametrize('fast_reader', [False, dict(use_fast_converter=False),
dict(use_fast_converter=True)])
@pytest.mark.parametrize("parallel", [False, True])
def test_data_out_of_range(parallel, fast_reader, guess):
"""
Numbers with exponents beyond float64 range (|~4.94e-324 to 1.7977e+308|)
shall be returned as 0 and +-inf respectively by the C parser, just like
the Python parser.
Test fast converter only to nominal accuracy.
"""
# Python reader and strtod() are expected to return precise results
rtol = 1.e-30
# Update fast_reader dict; adapt relative precision for fast_converter
if fast_reader:
fast_reader['parallel'] = parallel
if fast_reader.get('use_fast_converter'):
rtol = 1.e-15
elif np.iinfo(np.int_).dtype == np.dtype(np.int32):
# On 32bit the standard C parser (strtod) returns strings for these
pytest.xfail("C parser cannot handle float64 on 32bit systems")
if parallel:
if not fast_reader:
pytest.skip("Multiprocessing only available in fast reader")
elif TRAVIS:
pytest.xfail("Multiprocessing can sometimes fail on Travis CI")
fields = ['10.1E+199', '3.14e+313', '2048e+306', '0.6E-325', '-2.e345']
values = np.array([1.01e200, np.inf, np.inf, 0.0, -np.inf])
with catch_warnings(AstropyWarning) as w:
t = ascii.read(StringIO(' '.join(fields)), format='no_header',
guess=guess, fast_reader=fast_reader)
if fast_reader and not parallel: # Assert precision warnings for cols 2-5
> assert len(w) == 4
E assert 3 == 4
E + where 3 = len([<warnings.WarningMessage object at 0xe8d488b0>, <warnings.WarningMessage object at 0xe8d48538>, <warnings.WarningMessage object at 0xe8d48c10>])
astropy/io/ascii/tests/test_c_reader.py:1067: AssertionError
The same happens for False-fast_reader2-False.
Activity
dhomeier commentedon Nov 27, 2019
What system and compiler is this?
When you run
could you check which column does not raise a warning and what is read in from that col?
olebole commentedon Nov 27, 2019
That is Debian unstable, gcc 9.2.1.
pllim commentedon Nov 27, 2019
@taldcroft , any ideas?
dhomeier commentedon Nov 27, 2019
Hmm, the stable gcc version is 9.2.0, and apparently there haven't been any gnu.org "subsubversion" releases since 4.9.4; I'm wondering what the official status of Debian's 9.2.1 is...
dhomeier commentedon Nov 27, 2019
But then this apparently pops up in our hand-coded
xstrtod, not the builtin one.AFAICS the difference to the other input numbers is that
2048e+306will not already seterrno = ERANGEon the exponent (which by itself is still in the safe range), but only right afterwards on comparison of the converted(number == HUGE_VAL || number == -HUGE_VAL)– as
numberobviously isinf, I am puzzled why that comparison is not true. Maybe it now counts asinf > HUGE_VAL...?Could you change tokenizer.c to
if (number >= HUGE_VAL || number <= -HUGE_VAL)and check what happens?
olebole commentedon Nov 27, 2019
That seems to work now:
pllim commentedon Nov 27, 2019
Does this mean there needs to be a patch for
tokenizer.c? Thanks for looking into this, @dhomeier !dhomeier commentedon Nov 27, 2019
The above would be an easy enough fix. Just wondering if this is really conforming to standard, or a compiler bug after all. @olebole would you mind also testing
if (number == INFINITY || number == -INFINITY)?That appears to be the more up-to-date way to do it, although by all accounts the two seem to be supposed to work quite interchangeably.
olebole commentedon Nov 27, 2019
@dhomeier No --
INFINITYdoes not work. I could (wildly, without any competence) guess thatdoublevalues in registers on i386 are 80bit (not 64); so they may be internally still finite but converted to infinity when put into storage cells (depending on compiler optimization). That would explain why this does not happen on other 32 bit platforms: arm and mips build nicely.olebole commentedon Nov 27, 2019
The gcc-9
debian/changelogstates for the origin:dhomeier commentedon Nov 27, 2019
Well, for 32 bit platforms the
fast_readerversions of the test are markedxfailanyway, but the 80 bit register length could come in here indeed. One further variant to test would beif (number == __builtin_inf() || number == -__builtin_inf())– maybe using the builtin functions instead could indeed let the whole comparison run in the register. But I am somewhat out of my depth here too, at least I could not identify anything obviously related in the tracker log.
dhomeier commentedon Nov 27, 2019
Come to think, even if that comparison to
__builtin_inf()works with the 80 bit-values, it would take something larger than2.048e309to trigger it. But that's not what we actually want to know anyway - I think we want a warning whenever loss of precision is encountered on storing that value as anp.float64. Sonumber >= HUGE_VALis probably the way to go - at least unless and until we somehow manage to extract that number at the full 80 bit precision and store it asnp.float128...pllim commentedon Dec 3, 2019
So, is there a clear path forward to resolve this for RC2?
dhomeier commentedon Dec 3, 2019
Unless there are still concerns that @olebole's diagnosis might somehow be wrong and this a gcc 9.2.1 bug after all, I propose to go for changing the comparison to
>= HUGE_VAL.