desihub / desisim

DESI simulations
BSD 3-Clause "New" or "Revised" License
16 stars 22 forks source link

Smoothing source contribution to noise in quickquasars DESI mocks #566

Closed p-slash closed 2 years ago

p-slash commented 2 years ago

There is now an option in quickquasars to smooth out the source contribution to noise for DESI mocks in order to de-bias the weights in Lya forest analyses. The value passed will the Gaussian sigma for the smoothing in Angstrom. This smoothing is implemented with FFTs on a padded array in quickspectra.py.

p-slash commented 2 years ago

Sorry, this is the first time I'm seeing this. This is a subtle point. If we were to have the exact flux and noise, but different reported variance, that would be wrong. That is just the wrong noise estimate for that realization.

The point is to decouple the generated noise from the source flux, so they will be different for different smoothing scales. In this implementation, we smooth the variance first (only the source contribution) and generate noise from this smoothed variance. This way, the generated noise is decoupled from the source flux, and we still know the exact noise that is put in. Furthermore, using these reported variances as inverse weights will be unbiased (less biased) in statistics estimations. If we were to smooth out the generated noise, then it is hard to estimate the underlying variance.

In terms of P1D and Xi3D estimation, I think we should still smooth the noise in data and mocks, but only in its contribution to weights. Noise power subtraction should use the exact reported variance to correctly remove the noise power. The difference is that here we are smoothing out all contributions to noise, whereas the implementation in quickquasars smoothes out only the source contribution.

andreufont commented 2 years ago

@julienguy , could you comment on this? I thought that what I suggested above is what the real pipeline does.

andreufont commented 2 years ago

@julienguy , @p-slash - we forgot to discuss this at the telecon yesterday. Should we have a quick call to make sure we all agree on this? Do you prefer to discuss this online?

andreufont commented 2 years ago

Also, @p-slash - can you move the branch to the desisim repo, instead of your own fork? César Ramírez would look at it in the context of BAO analyses.

p-slash commented 2 years ago

Not sure how. I can try. Can't he pull from my branch to his local repo?

I would be fine with a chat or over text on Slack. I don't check here as often:)

andreufont commented 2 years ago

@p-slash , @julienguy - any update on this discussion?

julienguy commented 2 years ago

I commented we should change the variance model after generating the noise. Naim @p-slash I can make the change if you are not sure how to do this.

p-slash commented 2 years ago

I moved the function after sim.generate_random_noise(random_state,use_poisson=use_poisson). That should do the trick. I HAVE NOT tested it though. Sorry I've been busy with other things.

andreufont commented 2 years ago

@p-slash , @julienguy , @alxogm - what is the status of this PR?

alxogm commented 2 years ago

Sorry, I missed to follow this PR. Also I think we haven't tested the effect on the analysis. I can do early next week.

p-slash commented 2 years ago

Thanks Alma

alxogm commented 2 years ago

Hi @p-slash I have tested the branch and works well it gives the same flux and a smoothed version of the same noise for a given spectra . I was planning to run the deltas and CF but given the presentation today by Andrei, I think you can go ahead and merge.

I have only requested to add one line to run log to inform the smoothing was applied.

p-slash commented 2 years ago

Given our discussion at the workshop, should we change the default for --source-contr-smoothing to 10 A? This would mean behavior change for quickquasars moving forward. Also, @alxogm not sure where to add the line for logging.