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

Wrong redshift of absorber in fast_metal_dmat #1044

Open
cramirezpe opened this issue Oct 18, 2023 · 10 comments
Open

Wrong redshift of absorber in fast_metal_dmat #1044

cramirezpe opened this issue Oct 18, 2023 · 10 comments
Assignees

Comments

@cramirezpe
Copy link
Contributor

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
Copy link
Contributor

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

@andreufont
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

calumgordon commented Oct 19, 2023

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
Copy link
Contributor

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
Copy link
Contributor

calumgordon commented Oct 19, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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

@andreufont
Copy link
Contributor

@calumgordon , @julienguy - was this fixed / understood? If so, could we close the issue?

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

No branches or pull requests

3 participants