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

Fix sngls_minifollowup event selection/ordering [v23_release_branch] #4824

Conversation

GarethCabournDavies
Copy link
Contributor

We noticed that the sngls_minifollowup pages had essentially randomly chosen, and randomly-ordered events.

This was due to a bug introduced by an overzealous approach to the scope creep part of #4549 (my fault!)

This will also be fixed on the master branch in #4823

Standard information about the request

This is a bug fix
This change affects the offline search

This change changes result presentation

This change follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines

Contents

Return to using the mask_to_n_loudest_clustered_events function rather than trying anything too fancy

Testing performed

Currently running given latest version

  • Run pycbc_sngl_minifollow and check by printing values that the top events are in the right order, and that the are the highest-statistic events

  • The author of this pull request confirms they will adhere to the code of conduct

@GarethCabournDavies GarethCabournDavies added the v23_release_branch PRs applied to the v2.3.X release branch or to be cherry-picked if merging to master label Jul 25, 2024
Copy link
Contributor

@spxiwh spxiwh left a comment

Choose a reason for hiding this comment

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

If this works, it's fine with me. Tests are not passing though.

@GarethCabournDavies GarethCabournDavies merged commit 69874f0 into gwastro:v23_release_branch Aug 8, 2024
31 checks passed
@GarethCabournDavies GarethCabournDavies deleted the v23_singles_minifollow_order branch August 8, 2024 13:08
@spxiwh
Copy link
Contributor

spxiwh commented Sep 25, 2024

I'm assuming from commit history and notes that this is on the v23 release branch. Please comment if not.

@spxiwh spxiwh removed the v23_release_branch PRs applied to the v2.3.X release branch or to be cherry-picked if merging to master label Sep 25, 2024
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.

2 participants