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

Improvement to the fast metal matrix computation #1073

Merged
merged 8 commits into from
Sep 16, 2024
Merged

Conversation

julienguy
Copy link
Contributor

The current fast metal matrix computation ignores the change of r_perp . It's a small correction for contaminants such as the cross-correlation of Lya with SiII(1260A) ( 1.4% change of rt , which is negligible effect given that the correlation is concentrated at small rt), but it's larger for foreground contaminants such as CIV.

In this PR I implement the fast computation of the distortion with r_perp. Distortion matrices for r_par and r_perp are computed separately. The full metal matrix elements are the product of both. It works because each matrix acts only on the sub index corresponding to r_par or r_per.

Validation with comparison with the "brute force" method (picca_metal_dmat.py):

First Lya x SiII(1260A) metal matrix along the line of sight (first rt bins) as a function of rp (the true input correlation is xi(rp<4)=1 and xi(rp>4)=0 :

LYA_SiII_1260

Then CIV x CIV metal matrix transverse to the line of sight (rp<4) for different values of rt_true:

CIV_eff_CIV_eff

(This method will be presented during a DESI WG meeting.)

The effect on the fit of the Lya auto and Lya-QSO cross-correlation is almost completely negligible, but it seems to me that it's important to have correct metal matrices in the code ...

@iprafols
Copy link
Collaborator

Quick question before merging, are we automatically testing these changes?

@Waelthus
Copy link
Contributor

I didn't see it tested yet. So added 5 tests along the same lines as the slow version of the routines, using currently one of the outputs of delta_extraction as input (for test independence one should probably move that test data to the same dir as for the other tests).
So some testing code is there now within the framework of the other correlation tests, probably could be optimized...

@Waelthus
Copy link
Contributor

The test touches ~90% of the code in the new files with the rest just being related to blinding, rebinning, or a wrongly set input delta-dir. So copied the test files over to the right input dir for stability and then test-wise I think this could be merged...

@iprafols iprafols added this pull request to the merge queue Sep 16, 2024
Merged via the queue into master with commit 3bf05b3 Sep 16, 2024
10 checks passed
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

Successfully merging this pull request may close these issues.

3 participants