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

Possible float shenanigans in wavelength unit conversions #470

Open
teutoburg opened this issue Sep 24, 2024 · 0 comments
Open

Possible float shenanigans in wavelength unit conversions #470

teutoburg opened this issue Sep 24, 2024 · 0 comments

Comments

@teutoburg
Copy link
Contributor

This unit conversion could indeed go wrong. However, it already would go wrong if fractional wavelengths are used, e.g. wavemin=1333.3. Our code should not depend on float values being integer.

This means that even if we would hardcode this 1E4 here (and similarly elsewhere), that the code will still be broken in general. So we should not use such hacks and instead just let astropy handle it, as you did.

Per extension, it should not cause any problems if a value of 1000.0 becomes 999.9999999999 or 1000.000000001:

  • Any (synphot) code that computes overlaps should be resilient to such rounding errors and just ignore it.
  • Any plotting/printing code should just print 1000.0. (At least for UX, not for debugging.)

Perhaps we should find some floating point numbers that you cannot faithfully multiply/divide by (e.g.) 1E4. Maybe one number that rounds up and one that rounds down. Then we should use those in the basic_instrument test package to detect any errors that such a conversion causes. Or maybe have some test irdb packages that uses weird units like wavelengths in parsec and such.

Originally posted by @hugobuddel in #405 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📋 Backlog
Development

No branches or pull requests

1 participant