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.
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:
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 https://github.com/AstarVienna/ScopeSim/pull/405#discussion_r1741879120_