Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

multi-ifo changes to pycbc_page_segtable #2519

Merged
merged 6 commits into from
Feb 27, 2019

Conversation

GarethCabournDavies
Copy link
Contributor

Making it so that H1 & L1 are not longer hardcoded, and that the table output now has all possible detector combinations

@tdent
Copy link
Contributor

tdent commented Feb 22, 2019

Can you post links to some tests Gareth?

@GarethCabournDavies
Copy link
Contributor Author

GarethCabournDavies commented Feb 22, 2019

Segtable with 3 IFOs and multiple rows to check:

https://ldas-jobs.ligo.caltech.edu/~gdavies/testoutput/test_segtable_output.html

I'm not sure how this will look on the actual results page though, as it is now a much wider table

Edit:
as a test of backwards-compatibilty, here is the two-ifo table:
https://ldas-jobs.ligo.caltech.edu/~gdavies/testoutput/test_segtable_output_2only.html

@tdent
Copy link
Contributor

tdent commented Feb 22, 2019

@spxiwh OK on the output from my point of view, however the functions to iterate over ifo combinations (ifo_combinations and powerset_ifos) seem like they might be more generally useful or maybe already exist somewhere - we're guessing you may know more about this

@tdent
Copy link
Contributor

tdent commented Feb 25, 2019

@spxiwh can you look at this/review briefly? the one issue I had is whether the powerset_ifos function either exists elsewhere already or belongs in a library module, eg in the workflow module ?

@GarethCabournDavies
Copy link
Contributor Author

GarethCabournDavies commented Feb 27, 2019

looking at https://ldas-jobs.ligo.caltech.edu/~gdavies/testoutput/H1L1-PAGE_SEGTABLE_VETO_SUMMARY-1187312718-428100.html in #2509 has made me want to have a look at this again at some point, need to understand if this is doing this right

I'm not sure if V1 segments are in a dfferent format and therefore need special treatment

(Fixed by 5fd82e9, see https://ldas-jobs.ligo.caltech.edu/~gdavies/testoutput/H1L1-PAGE_SEGTABLE_VETO_SUMMARY-1187312718-428100-V2.html)

Copy link
Contributor

@tdent tdent left a comment

Choose a reason for hiding this comment

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

We will ignore the powerset_ifos question (git grep powerset gives nothing) & merge when ready

@tdent tdent self-requested a review February 27, 2019 16:08
@tdent tdent merged commit 6108456 into gwastro:master Feb 27, 2019
@GarethCabournDavies GarethCabournDavies deleted the multiifo_page_segtable branch March 6, 2019 13:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants