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

PyGRB: Should compute_new_distance be kept as .py code or add to some class ? #4591

Open
Prasia-Pankunni opened this issue Dec 18, 2023 · 5 comments
Assignees
Labels
PyGRB PyGRB development

Comments

@Prasia-Pankunni
Copy link
Contributor

The code https://github.com/Prasia-Pankunni/pycbc/blob/fermi_sky_dist/pycbc/inject/compute_injected_distance.py compute the offset for the distances after/before computing offsets to the ra-dec. I am not sure where in the new PyGRB workflow this is called for. This code computes the new injected distances based on the amplitude and phase error (for a given d-dist ). My doubt is whether to keep the code as it is or add on to some other appropriate class.

@titodalcanton
Copy link
Contributor

titodalcanton commented Feb 20, 2024

Hi @Prasia-Pankunni, I just had a quick look at this.

Looking at the code you linked, I think this functionality could be realized via the waveform_transform functionality of pycbc_create_injections. Basically you draw your astrophysical injection population using a true luminosity distance parameter, and then you use a waveform transform to map the true distance to the jittered distance according to the desired amplitude error, phase error and jittering distribution. In this case all you have to do is add a new class to pycbc/transforms.py that applies the right tranformation.

This would not handle the effective distance fields, but are they really necessary?

This would be a great solution if it works, because it would not require any new executable, the calibration parameters would be clearly defined within a section of the injection definition, and the same configuration would apply to every code that uses injection files, including the all-sky offline search.

@pannarale
Copy link
Contributor

This sounds like a reasonable approach to me and an option worth investigating. @spxiwh, before @Prasia-Pankunni starts looking into this, does placing the jittering code capabilities in pycbc/transforms.py make sense to you as well?

@spxiwh
Copy link
Contributor

spxiwh commented Mar 28, 2024

Apologies, I missed this! How would you know what the original distances etc. are if doing this? I'm also not sure it would let you independently jitter distance/phase/etc. in H1 vs L1 vs V1, which is really needed.

@titodalcanton
Copy link
Contributor

Aha, I think @spxiwh has a good point about the original distances. Going through a separate "jittering" executable allows us to have both the original and jittered injections. Using a waveform transform, we would only have the jittered injections.

@ahnitz
Copy link
Member

ahnitz commented Mar 28, 2024

@titodalcanton You could have both the original and jittered distance in the same injection file. They just have to have different names. I think the only thing that I don't see how to do is to apply a different factor for each detector for a single injection e.g. H1 is multiplied by 1.01 while at the same time L1 is multiplied by .99.

@spxiwh It doesn't look the script is actually doing anything independent between ifos though. Maybe it's eventually supposed to though?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PyGRB PyGRB development
Projects
Status: In Progress
Development

No branches or pull requests

6 participants