igmhub / picca

set of tools for continuum fitting, correlation function calculation, cosmological fits...
GNU General Public License v3.0
30 stars 22 forks source link

Wrong redshift of absorber in fast_metal_dmat #1044

Open cramirezpe opened 9 months ago

cramirezpe commented 9 months ago

There's an issue in the new picca_fast_metal_dmat that is causing the redshift of the absorbers to be at a smaller redshift than it should.

Calum just shared this plot: image The difference in number of points is probably due to the use of higher resolution in the fast one.

We still need to check the xdmat case.

andreufont commented 9 months ago

@calumgordon - could you check what happens with the ~500 bins in the blue histogram that have z_eff_civ=0?

andreufont commented 9 months ago

About the total number of point in that histogram, shouldn't these matrices have 2500^2 elements, i.e., (50x50)^2? This would be 6250000 (>6M) but I don't see that many points in the histogram...

calumgordon commented 9 months ago

It's the CIVxCIV correlation in the metal dmat, which has 150x100 bins in the fast case, and 75x50 in the slow.

For the bins with z_eff = 0, there isn't a lot of info easily available, so I'm not sure.

calumgordon commented 9 months ago

WDM

The sum of weights in the z_eff=0 bins is higher on average, so I don't know what the bug is.

calumgordon commented 9 months ago

Ok, here is a plot of RT_CIV, where we can see the missing bins from 176 to 200 Mpc/h. I've also checked and this indeed corresponds to (200-176)/4 * 75 = 450 bins.

(I don't know why the axes are not visible on these plots)

RT_CIV

andreufont commented 9 months ago

This is soooo confusing. I first thought that it would be a few rt bins that happens to not be covered because of the 0.1% of data used in the computation of matrices, but the fact that they are all localised at rt > 175 Mpc/h means that this is more likely a bug somewhere... It would be quite a coincidence.

My next guess is that somewhere in the code, one uses the CIV redshift (and not the Lya) to compute maximum angular separation to include in the search for neighboring quasars, and that this cause quasars at > 175 Mpc/h (in Lya coordinates) to not be included because they correspond to > 200 Mpc/h in SiIV coordinates

calumgordon commented 9 months ago

RT_LYA - RT_CIV vs RT_LYA. There is a difference between the co-ordinates as expected but it's quite small (it goes from y ~ 0.25 to 0, then starts to trend towards y ~ 1, if you can't read the axes) rt_diff_lya_civ

andreufont commented 9 months ago

I don't think this makes sense, but at this point I have given up on understanding how we the old code works.

I need to go back to the blackboard and write down a simple example to make sure I understand how it works.

andreufont commented 8 months ago

@calumgordon - have looked at this any further? It would be good to (eventually) understand these values (for both the old and new metal implementation)