Skip to content

Conversation

@olebole
Copy link
Member

@olebole olebole commented Sep 21, 2019

On 32-bit x86, test_template_match_multidim_spectrum fails during build due to slightly different floating points:

    def test_template_match_multidim_spectrum():
        """[…]"""
        np.random.seed(42)
    
        # Create test spectra
        spec_axis1 = np.linspace(0, 50, 50) * u.AA
        spec_axis2 = np.linspace(0, 50, 50) * u.AA
    
        spec = Spectrum1D(spectral_axis=spec_axis1,
                          flux=np.random.sample(50) * u.Jy,
                          uncertainty=StdDevUncertainty(np.random.sample(50)))
        multidim_spec = Spectrum1D(spectral_axis=spec_axis2,
                                   flux=np.random.sample((2, 50)) * u.Jy,
                                   uncertainty=StdDevUncertainty(np.random.sample((2, 50))))
    
        # Get result from template_match
        tm_result = template_comparison.template_match(spec, multidim_spec)
    
>       assert tm_result[1] == 250.26870401777543
E       assert 250.26870401777546 == 250.26870401777543

The equal here should be an "almost equal". This patch fixes this in test_template_comparison.py. Although there are direct comparisons in many other test files, I pragmatically left them untouched since they didn't cause a failure, and I am not always sure whether a direct equal was meant there.

Copy link
Member

@eteq eteq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this is definitely better. Thanks @olebole!

Longer term it might be better to replace these tests with something that's more heuristic anyway (i.e. a "is the chi squared better in this case versus that other one" instead of an exact numerical test), but this is definitely an improvement.

@eteq eteq merged commit d729107 into astropy:master Sep 23, 2019
@eteq
Copy link
Member

eteq commented Sep 23, 2019

cc @javerbukh as an FYI to just for future related development on the redshift-finding machinery.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants