Do better with waveranges

[hugobuddel]

There were loads of irrelevant warnings being spit out, prevent those by dealing better with floats.

[codecov]

Codecov Report

❌ Patch coverage is 60.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.16%. Comparing base (e386ea5) to head (6b8bcbf).

Files with missing lines	Patch %	Lines
scopesim/optics/optical_train.py	50.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #936      +/-   ##
==========================================
- Coverage   75.17%   75.16%   -0.02%     
==========================================
  Files          70       70              
  Lines        9024     9030       +6     
==========================================
+ Hits         6784     6787       +3     
- Misses       2240     2243       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[teutoburg]

Yeah floats can be nasty. I've added countless round over the years, but it seems there's always more.

[hugobuddel]

Oh, I should make sure the test don't fail though

[teutoburg]

Oh, I should make sure the test don't fail though

Yeah, for some reason I didn't see the failures on the app? I do now, whatever. Once everything passes, go ahead and merge.

[hugobuddel]

The tests were useful but annoying.

See,

np.arange(209775, 210225, 90)

indeed does not include the end point

array([209775, 209865, 209955, 210045, 210135])

But divide everything by 10000, and this happens:

np.arange(2.09775, 2.10225, 0.0009)
array([2.09775, 2.09865, 2.09955, 2.10045, 2.10135, 2.10225])

where the last two numbers are actually 2.1013500000000005 and 2.1022500000000006.

Hmm is that correct behavior from numpy actually? To have arange end with a number that is higher than the given end point?

Anyway it works now. But I saw the same code is used later in the same function, so I updated that too, but that is apparently untested.

[oczoske]

We do have a function sim.utils.seq that fixes some of the nonsense that np.arange produces.

[hugobuddel]

We do have a function sim.utils.seq that fixes some of the nonsense that np.arange produces.

Weirdness indeed, but at least it is somewhat documented: https://numpy.org/doc/stable/reference/generated/numpy.arange.html

The length of the output might not be numerically stable.

with some bizar examples.

Thanks for the seq suggestion, maybe that would work here. But not necessarily, because seq includes the endpoints. That is

seq(2.09775, 2.10225, 0.0009)

includes 2.10225, but increase the step a bit and the end point would not be included (because it would not fit). The logic I added was to always add the endpoint as the last point, which would not work as-is with seq (but then again, arange includes points beyond the endpoint, which is worse.)

Maybe we should internally do everything in u.AA and only ever convert to something else at the boundaries.

Or use linspace as the numpy manual suggests.

I'm not sure what the best way to proceed is. Probably to improve the code now, but I don't want to; the patch works...

[hugobuddel]

Maybe we should internally do everything in u.AA and only ever convert to something else at the boundaries.

Here I meant to only allow integer ångström wavelengths. So we don't need to do any float conversion, just do everything in ints internally.

[teutoburg]

Or use linspace as the numpy manual suggests.

That would be my gut instinct option...

Maybe we should internally do everything in u.AA and only ever convert to something else at the boundaries.

Here I meant to only allow integer ångström wavelengths. So we don't need to do any float conversion, just do everything in ints internally.

Synphot "does everything in AA internally" anyway, and since we depend on it in many places, I think it wouldn't be the worst idea to also do that. Input/output to the user can still be in µm, if everything is in Astropy units that's easy enough...

[teutoburg]

I don't quite understand this line. You're checking for the absolute values of the equality check? Wouldn't that always be a bool?

[teutoburg]

Now that this takes 4 lines and occurs twice, maybe consider wrapping it in a utils function?