Optimize find bins fix issue 993 #1000
Conversation
|
Note that tests are failing due to some updates of numba and/or numpy in the github actions server (and are unrelated to the changes here) |
|
I think it's #1001. I'll have a look through all positions where this happens and will open another PR |
| w1 &= log_lambda >= log_lambda_grid[0] - half_pixel_step | ||
| w1 &= log_lambda < log_lambda_grid[-1] + half_pixel_step | ||
| w1 &= (log_lambda - np.log10(1. + z) >= | ||
| log_lambda_rest_frame_grid[0] - half_pixel_step_rest_frame) | ||
| log_lambda_rest_frame_grid[0]) |
There was a problem hiding this comment.
Did we change the definition of the rest frame grid from pixel centers to edges or vice versa? Or was it the effect of different find_bins implementation?
Or is this part of the reason we're seeing slightly different outputs?
| else: | ||
| break | ||
| step = grid_array[1] - grid_array[0] | ||
| found_bin = ((original_array - grid_array[0]) / step + 0.5).astype(np.int64) |
There was a problem hiding this comment.
Wouldn't this give an array that is displaced by exactly half a pixel and then cast to int? Is this for avoiding numerical issues from the division? Why not just use (original - grid) // step in that case?
| 3.01953334, 3.02453334, 3.02953334, 3.03453334, 3.03953334, | ||
| 3.04453334, 3.04953334, 3.05453334, 3.05953334, 3.06453334, | ||
| 3.06953334, 3.07453334 | ||
| 3.01703334, 3.02203334, 3.02703334, 3.03203334, 3.03703334, |
There was a problem hiding this comment.
this is a change by exactly 0.0025, i.e. one half pixel, did we want to change the definition? see other file
This PR fixes issue #993 and optimized function find_bins to use direct calculation. The results are different since I changed the binning of the wavelength grids in the computation of the expected flux. Thus, I updated the test suite. Below I provide a comparison of the results of this branch with those from master with a fugu run.
Timing comparison (over 3 runs):
Variance statistics comparison:
Delta stack comparison:
Quasar continuum comparison:
Fits metadata comparison:
Lya autocorrelation comparison:
