Skip to content

Adding functionalities to create calibration config files#4537

Merged
sum33it merged 23 commits intogwastro:masterfrom
sum33it:calibration_uncertainties_files
Jul 16, 2025
Merged

Adding functionalities to create calibration config files#4537
sum33it merged 23 commits intogwastro:masterfrom
sum33it:calibration_uncertainties_files

Conversation

@sum33it
Copy link
Copy Markdown
Member

@sum33it sum33it commented Oct 18, 2023

To incorporate calibration uncertainties in PE runs, one need to provide a calibration configuration files to pycbc_inference code. Although, the support for the calibration config file exist, we still do not have a way to generate these config files, within PyCBC infrastructure. In this PR, I include the full infrastructure to generate the config files for PE runs for any GPS time from the publicly available calibration envelopes provided by LIGO-Virgo.

@ahnitz
Copy link
Copy Markdown
Member

ahnitz commented Dec 13, 2023

@sum33it I don't think there are any strong objections to this. Maybe you can add a short example of using this? If that works, then I think we can go ahead and merge.

@ahnitz
Copy link
Copy Markdown
Member

ahnitz commented Mar 12, 2024

@sum33it What's the status of this? Can we get this rebased and then merged?

@sum33it sum33it force-pushed the calibration_uncertainties_files branch from 2ffc6b1 to f80c68f Compare March 19, 2025 19:44
@sum33it sum33it force-pushed the calibration_uncertainties_files branch 3 times, most recently from cb525d2 to d5f7a07 Compare April 14, 2025 17:05
@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented Apr 15, 2025

@ahnitz I think this PR is ready for the review.

@ahnitz
Copy link
Copy Markdown
Member

ahnitz commented Apr 15, 2025

@sum33it Can you add an example to the documentation? I think in this case, that would be very useful and help verify the code continues to work.

@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented Apr 15, 2025

I will open a follow-up PR for detailed documentation and examples. But here, I have added an example script of how to generate a calibration config file for GW150914. The extension to other events is trivial: one needs to provide appropriate arguments like ifo-list and trigger-time.

@sum33it sum33it requested a review from mj-will May 6, 2025 16:02
@sum33it sum33it force-pushed the calibration_uncertainties_files branch from 766b520 to 32f811e Compare May 12, 2025 12:40
Copy link
Copy Markdown
Contributor

@mj-will mj-will left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had a look over the new function for reading the calibration envelopes and added a few minor comments.

Comment thread pycbc/strain/recalibrate.py
Comment on lines +565 to +566
else:
pass
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, it might not be needed, but I still want to keep it in case we go beyond the frequency range of the calibration provided by the calibration envelopes. For example, if someone wants to do PE with a lower cutoff frequency than the standard, they might be tempted to give the same f_lower for calibration config file generation. This option should safeguard against that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, just to clarify, I mean then else part of the if/else statement. I think it's a good idea to keep the checks you implemented.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Thanks for the clarification. I agree with that. Will do in the following commit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mj-will I have made the necessary changes and pushed the new commit.

Comment on lines +571 to +572
else:
pass
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented May 20, 2025

Hi @mj-will
Thank you for the review. I have made appropriate changes. Could you please check if you are happy with this?

@sum33it sum33it requested a review from mj-will May 21, 2025 14:17
Copy link
Copy Markdown
Contributor

@mj-will mj-will left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sum33it for addressing my comments. I'm happy to approve this (assuming the tests pass and someone which the relevant expertise reviews the executable.)

@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented May 22, 2025

Thanks a lot @mj-will .
@ahnitz if you don't have objection, shall we merge this PR?

@ahnitz
Copy link
Copy Markdown
Member

ahnitz commented May 22, 2025

@sum33it The unit tests need to pass. That's why I said to rebase, then see if anything needs to be addressed there. Otherwise, I don't have other objections.

@sum33it sum33it force-pushed the calibration_uncertainties_files branch 2 times, most recently from d5f7a07 to 849a4c8 Compare May 22, 2025 22:39
@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented May 23, 2025

@ahnitz I have rebased it and there is one failed test and it seems that it is not originating from my commits. What do you suggest?

@GarethCabournDavies
Copy link
Copy Markdown
Contributor

@sum33it Try rebasing to gwastro:master and starting the checks again - they've expired now so I can't check the run failures, but I will take a look at them if it fails again

@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented Jul 10, 2025

@GarethCabournDavies The two failed tests are related to i) py-docs, and in ii) retrieving frame data with bash -e examples/search/get.sh. I don't see any of them related to my commits.

@GarethCabournDavies
Copy link
Copy Markdown
Contributor

I think the sphinx documentation failure is real - when I try and run the examples/inference/generate_calibration_config.sh script, I am getting an error:

Traceback (most recent call last):
  File ".../bin/pycbc_inference_create_calibration_config", line 127, in <module>
    calibration_files_dict = get_calibration_files_O1_O2_O3(opts.ifos, gps_time, calibration_files_path)
  File ".../pycbc/pycbc/strain/recalibrate.py", line 610, in get_calibration_files_O1_O2_O3
    ifo_calibration_file = '%s/%s'%(os.getcwd(), all_calibration_files[dt_list.argmin()])
ValueError: attempt to get argmin of an empty sequence

The documentation will run the examples in the examples/inference folder, but it currently doesn't give the failure / output nicely

@GarethCabournDavies
Copy link
Copy Markdown
Contributor

(which makes sense as it hardcodes files in your own path)

Copy link
Copy Markdown
Contributor

@GarethCabournDavies GarethCabournDavies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes should work to make the example run locally.

As an aside, it would neaten up the code if you could change print statements to logging, and use the pycbc.add_common_pycbc_options and pycbc.init_logging functions, but no worries if you can't.

Comment thread examples/inference/calibration_configuration_files/generate_calibration_config.sh Outdated
sum33it and others added 2 commits July 11, 2025 13:50
…libration_uncertainty_files_o1_o2_o3.sh

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>
…libration_config.sh

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>
@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented Jul 11, 2025

Thanks @GarethCabournDavies I implemented your changes in the example folder and now we have only sphinx documentation related failure. Shall I just merge this at this point?

@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented Jul 11, 2025

@ahnitz Do you have any objection on merging it? Failed test seems to be coming from sphinx documentation failure which is not related to this PR.

@spxiwh
Copy link
Copy Markdown
Contributor

spxiwh commented Jul 12, 2025

The documentation failure is related to this patch. It must be fixed before this can be merged:

/home/runner/work/pycbc/pycbc/.tox/py-docs/lib/python3.11/site-packages/pycbc/strain/recalibrate.py:docstring of pycbc.strain.recalibrate.get_calibration_files_O1_O2_O3:6: WARNING: Title underline too short.

@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented Jul 13, 2025

Thanks a lot @spxiwh for pointing this out. I have fixed that and now there is one skipped test related to deploy_documentation. I can't see it being related to my PR. I hope it is OK to merge this PR now.

@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented Jul 15, 2025

@spxiwh, @GarethCabournDavies , @ahnitz: If you have no further objections, shall we merge it now?

Copy link
Copy Markdown
Contributor

@GarethCabournDavies GarethCabournDavies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can got for it merging for me - Ive made a couple of suggestions for using logging rather than printing.

It looks like one of the print statements should be an error. You can change this to logging.warning if preferred though

Comment thread bin/inference/pycbc_inference_create_calibration_config Outdated
Comment thread bin/inference/pycbc_inference_create_calibration_config Outdated
Comment thread bin/inference/pycbc_inference_create_calibration_config Outdated
Comment thread bin/inference/pycbc_inference_create_calibration_config Outdated
Comment thread bin/inference/pycbc_inference_create_calibration_config Outdated
sum33it and others added 5 commits July 15, 2025 17:54
Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>
Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>
Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>
Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>
Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>
@sum33it
Copy link
Copy Markdown
Member Author

sum33it commented Jul 16, 2025

Thanks @GarethCabournDavies for suggestions! I have accepted your suggested commits. I will merge it sometime today.

@GarethCabournDavies
Copy link
Copy Markdown
Contributor

Weirdly, this passed without #5156 ??

@sum33it sum33it merged commit ce197ac into gwastro:master Jul 16, 2025
31 of 32 checks passed
@sum33it sum33it deleted the calibration_uncertainties_files branch July 16, 2025 13:56
khunsang pushed a commit to khunsang/pycbc that referenced this pull request Jul 23, 2025
* WIP: Adding utilities to create calibration files`

* WIP: Added a binary script for creating calibration config file

* WIP: updating functions for writing calibration file.

* WIP

* WIP: Functions for calibration configuration

* WIP: Added functions to read envelop files

* Updated the scripts to create calibration files

* Added examples to generate config files

* Fixed a minor bug in the function to read calibration envelop files

* Updated the create_calibration_file for additional functionality

* Minor cleanup for unusual imports

* Update examples/inference/calibration_configuration_files/download_calibration_uncertainty_files_o1_o2_o3.sh

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Update examples/inference/calibration_configuration_files/generate_calibration_config.sh

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Docstring cleanup

* Fix for the unit tests fail

* Cleanup of the main executable file

* Cleanup for unit tests

* Cleanup

* Update bin/inference/pycbc_inference_create_calibration_config

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Update bin/inference/pycbc_inference_create_calibration_config

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Update bin/inference/pycbc_inference_create_calibration_config

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Update bin/inference/pycbc_inference_create_calibration_config

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Update bin/inference/pycbc_inference_create_calibration_config

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

---------

Co-authored-by: Sumit Kumar <sumit.kumar@condor5.atlas.local>
Co-authored-by: Sumit Kumar <sumit.kumar@condor4.atlas.local>
Co-authored-by: Sumit Kumar <sumit.kumar@holodeck1>
Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>
GarethCabournDavies added a commit to GarethCabournDavies/pycbc that referenced this pull request Oct 7, 2025
* WIP: Adding utilities to create calibration files`

* WIP: Added a binary script for creating calibration config file

* WIP: updating functions for writing calibration file.

* WIP

* WIP: Functions for calibration configuration

* WIP: Added functions to read envelop files

* Updated the scripts to create calibration files

* Added examples to generate config files

* Fixed a minor bug in the function to read calibration envelop files

* Updated the create_calibration_file for additional functionality

* Minor cleanup for unusual imports

* Update examples/inference/calibration_configuration_files/download_calibration_uncertainty_files_o1_o2_o3.sh

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Update examples/inference/calibration_configuration_files/generate_calibration_config.sh

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Docstring cleanup

* Fix for the unit tests fail

* Cleanup of the main executable file

* Cleanup for unit tests

* Cleanup

* Update bin/inference/pycbc_inference_create_calibration_config

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Update bin/inference/pycbc_inference_create_calibration_config

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Update bin/inference/pycbc_inference_create_calibration_config

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Update bin/inference/pycbc_inference_create_calibration_config

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

* Update bin/inference/pycbc_inference_create_calibration_config

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>

---------

Co-authored-by: Sumit Kumar <sumit.kumar@condor5.atlas.local>
Co-authored-by: Sumit Kumar <sumit.kumar@condor4.atlas.local>
Co-authored-by: Sumit Kumar <sumit.kumar@holodeck1>
Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>
@sum33it sum33it mentioned this pull request Dec 16, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants