SNR optimisation options for pycbc_live #4432

ArthurTolley · 2023-07-11T19:02:11Z

This is a recreation of #4398 which is now closed due to my mistake of creating the PR using Gareth's branch and then not being able to make changes to that branch.

This PR includes a new directory containing two new files:
pycbc/live -> __init__.py + snr_optimizer.py

snr_optimizer.py contains the functions required to do the snr optimization (the snr optimizers themselves, but not the boundary calculations)

The following two files have also been modified:
pycbc_live - includes the new snr_optimizer options group.
pycbc_optimize_snr - removing the functionality that has been moved to snr_optimizer.py

I am yet to fully test these changes and will do so pronto.

pycbc/live/snr_optimizer.py

ArthurTolley · 2023-07-18T08:48:34Z

Currently the example in examples/live/run.sh works with --optimizer pso but when using differential_evolution there is an issue with the command line options. Giving the two differential evolution options, maxiter and popsize, there is a bug when trying to run the optimizer which says:
pycbc_optimize_snr: error: unrecognized arguments: --differential_evolution-maxiter 50.0 --differential_evolution-popsize 100.0

I've looked at how the option group is generated and the option group looks fine, I'm thinking maybe there is an issue with how the option is being passed from the command line in to the program. Not sure and i can't figure it out.

ArthurTolley · 2023-07-18T08:50:02Z

Another comment, I've had to explicitly make the inputs to some of the functions within snr_optimizer ints because the option group creating function makes them all floats. I will try and modify this function to make them require ints for those functions otherwise the --help descriptions will be wrong.

GarethCabournDavies

As this is moving a lot of code into a new module, we will lose the blame information. So it is easy(ish) to find, here's the LINK

(It may be good to add this to the description)

pycbc/live/snr_optimizer.py

GarethCabournDavies · 2023-07-18T09:01:13Z

Currently the example in examples/live/run.sh works with --optimizer pso but when using differential_evolution there is an issue with the command line options. Giving the two differential evolution options, maxiter and popsize, there is a bug when trying to run the optimizer which says: pycbc_optimize_snr: error: unrecognized arguments: --differential_evolution-maxiter 50.0 --differential_evolution-popsize 100.0

Looks like the options above have underscores where they should have hyphens

ArthurTolley · 2023-07-18T09:02:48Z

@GarethCabournDavies They do but I don't know why they do. Surely unrecognized arguments means it's being given the wrong arguments but I'm giving it arguments which have hyphens and not underscores.

examples/live/run.sh

pycbc/live/snr_optimizer.py

tdent

Only a few formatting / comment requests

GarethCabournDavies · 2023-09-11T14:31:37Z

So far as I can tell, we just need someone to merge it - @ArthurTolley, is that right?

ArthurTolley · 2023-09-11T14:33:49Z

I made some more changes in response to comments made in some of the conversations still not marked 'resolved', mainly regarding naming conventions I believe. I think I did ask for one final review but it's been a while. If it all looks good then another ✅ would be all I need to hit the merge button.

I'm pretty sure it's all tested but if I get some time tomorrow I can re-do all the tests just to triple check.

tdent · 2023-09-12T14:28:19Z

Codeclimate has some weird looking error ..

warning: Could not find remote branch live_options to clone.
fatal: Remote branch live_options not found in upstream origin

ArthurTolley · 2023-09-12T14:44:37Z

That is a really weird error, as far as I can tell the branch should be available 🤷🏻‍♂️

bin/pycbc_optimize_snr

pycbc/live/snr_optimizer.py

examples/live/run.sh

spxiwh · 2023-09-12T15:12:02Z

CodeClimate is visible now, and it does need some placating :-)

spxiwh · 2023-09-12T15:12:58Z

... Although it's probably the case that most of these issues are from me, not from this patch. So maybe not fair to enforce that.

ArthurTolley · 2023-09-12T15:15:30Z

I did clean the codeclimate of any issues relating to this PR.

tdent · 2023-09-12T15:26:39Z

@spxiwh @ArthurTolley why are we using numpy.random.mtrand._rand ? Isn't there a more 'standard' way to get a random number?

tdent · 2023-09-12T15:27:21Z

Out of the remaining codeclimate whines, I think removing the unused argparse import is worth noting

GarethCabournDavies · 2023-09-12T15:32:33Z

@spxiwh @ArthurTolley why are we using numpy.random.mtrand._rand ? Isn't there a more 'standard' way to get a random number?

Thats from a previous PR - #4393, but its basically one of a few ways of getting the global random state (see https://stackoverflow.com/a/41985898)

tdent · 2023-09-12T15:38:31Z

(should be fixed now)

ArthurTolley · 2023-09-12T15:39:34Z

@tdent Thank you for that, I'm getting some strange problems with trying to use VSCode today and I'm really rusty using the terminal to do changes.

* Moving the live optimizer option changes to my own branch * Completing the snr optimization argument group * updating pycbc_live * re-adding bug fix * removing TODO message * Bug with d_e options * Adding optimizer-seed * fixing the d_e optimizer * replacing run.sh code * resolve merge conflict * fixing run.sh * cleaning up args_to_string func * changing comment * codeclimate fixes * module docstring * Update module docstring copyright Co-authored-by: Gareth S Cabourn Davies <[email protected]> * Add gareth * removing argv * argument changing * removing duplicated arguments * minor CC points * remove bug introduced when making CC happier --------- Co-authored-by: Gareth S Cabourn Davies <[email protected]> Co-authored-by: Thomas Dent <[email protected]>

@josh-willis

* Remove reference to relative_example in docs * Use RTLD_GLOBAL for libgomp (#4353) * Use RTLD_GLOBAL for libgomp In conda-forge/pycbc-feedstock#74 it was suggested to use RTLD_GLOBAL for libgomp. Let's see if this works fine with the test suite (which should answer @josh-willis 's concerns). * Move import of ctypes/gomp into __enter__ * Try this * Revert "Revert "Allow SNR optimizer to use candidate point in initial array (#4393)"" This reverts commit 7be12f1. We are now catching up with master, where the bug originally introduced by #4393 is fixed properly, so here I am undoing the temporary fix. * SNR optimisation options for pycbc_live (#4432) * Moving the live optimizer option changes to my own branch * Completing the snr optimization argument group * updating pycbc_live * re-adding bug fix * removing TODO message * Bug with d_e options * Adding optimizer-seed * fixing the d_e optimizer * replacing run.sh code * resolve merge conflict * fixing run.sh * cleaning up args_to_string func * changing comment * codeclimate fixes * module docstring * Update module docstring copyright Co-authored-by: Gareth S Cabourn Davies <[email protected]> * Add gareth * removing argv * argument changing * removing duplicated arguments * minor CC points * remove bug introduced when making CC happier --------- Co-authored-by: Gareth S Cabourn Davies <[email protected]> Co-authored-by: Thomas Dent <[email protected]> * Improvements to single-detector trigger fitting code for PyCBC Live (#4486) * Cleanup * Cleanup * Refactor duration bin parsing code and add support for reading from bank * Minor fix/cleanup to logging * Update CLI checks for duration bins * Cleanup * Ignore inconsistent config when combining * Fix bug * Fix typo Co-authored-by: Gareth S Cabourn Davies <[email protected]> * Comment from Gareth --------- Co-authored-by: Gareth S Cabourn Davies <[email protected]> * [pycbc live] Don't add snr options to command if they don't exist (#4518) * Don't run snr optimizer setup if not optimizing snr * moving the check to a more appropraite place * setting snr_opt_options to None if not optimizing * [pycbc live] Allowing the use of psd variation in the ranking statistic for pycbc live (#4533) * Modifying files to include psd variation in single detector statistic calculation * ending variation.py with a blank line * Changing to an increment agnostic solution * removing change already fixed * Updating function names and docstrings * removing ToDos and adding more helpful comments * Removing unused import * Codeclimate fixes * Removing excess logging and whitespace mistakes * Removing unused objects + codeclimate fixes * Updating comments and docstrings, removing matchedfilter changes * Revert "Updating comments and docstrings, removing matchedfilter changes" This reverts commit 0e6473a. * Removing matchedfilter changes, updating comments and docstrings * Move --verbose to the end of the commands * more comment updates * Repositioning filter recreation * Changes to comments and removing whitespace Co-authored-by: Thomas Dent <[email protected]> * removing refchecks * Adding option veification for psd variation * Apply suggestions from code review Co-authored-by: Thomas Dent <[email protected]> * fixing EOL error * Refactoring the filter creation function * codeclimate fixes * undo * full_filt func * removing indentation * code climate * code climate * try to quiet codeclimate * codeclimate doesn't know PEP8 * brackets obviate line continuation --------- Co-authored-by: Thomas Dent <[email protected]> * added scaling of initial pop in snr_optimizer (#4561) * added scaling of initial pop * init popn in optimize_di & pso func * added changes in optimize_pso * usig logging.debug for snr * Do not set matplotlib's backend in internal modules (#4592) * Set version to 2.1.4 * Remove reference to single_template_examples in docs * Remove reference to hierarchical_model in docs * Live: produce empty trigger fit plot for detectors with no triggers (#4600) * Live: produce empty trigger fit plot for detectors with no triggers * allow for below-threshold triggers * fix thinko in option parsing for defaults (#4615) * fix thinko in option parsing for defaults When an option is not given at all getattr on the args object gives None, but we don't want to translate that into "--option-name None" on the command line. * bugfix obviously we needed to define 'key_name' first .. * Improvements to single fit plots (#4509) * Improvements to single fit plots * Apply suggestions from Gareth Co-authored-by: Gareth S Cabourn Davies <[email protected]> --------- Co-authored-by: Gareth S Cabourn Davies <[email protected]> --------- Co-authored-by: Ian Harry <[email protected]> Co-authored-by: Arthur Tolley <[email protected]> Co-authored-by: Gareth S Cabourn Davies <[email protected]> Co-authored-by: Thomas Dent <[email protected]> Co-authored-by: Praveen Kumar <[email protected]>

* Moving the live optimizer option changes to my own branch * Completing the snr optimization argument group * updating pycbc_live * re-adding bug fix * removing TODO message * Bug with d_e options * Adding optimizer-seed * fixing the d_e optimizer * replacing run.sh code * resolve merge conflict * fixing run.sh * cleaning up args_to_string func * changing comment * codeclimate fixes * module docstring * Update module docstring copyright Co-authored-by: Gareth S Cabourn Davies <[email protected]> * Add gareth * removing argv * argument changing * removing duplicated arguments * minor CC points * remove bug introduced when making CC happier --------- Co-authored-by: Gareth S Cabourn Davies <[email protected]> Co-authored-by: Thomas Dent <[email protected]>

ArthurTolley added 5 commits July 11, 2023 19:24

Moving the live optimizer option changes to my own branch

f47c5fb

Completing the snr optimization argument group

09ea171

updating pycbc_live

71bc23c

re-adding bug fix

46ba8ab

removing TODO message

86a5107

ArthurTolley self-assigned this Jul 11, 2023

ArthurTolley mentioned this pull request Jul 11, 2023

SNR optimisation options for pycbc_live #4398

Closed

ArthurTolley assigned GarethCabournDavies Jul 11, 2023

tdent added the low latency label Jul 17, 2023

tdent reviewed Jul 17, 2023

View reviewed changes

pycbc/live/snr_optimizer.py Show resolved Hide resolved

tdent reviewed Jul 17, 2023

View reviewed changes

pycbc/live/snr_optimizer.py Show resolved Hide resolved

tdent reviewed Jul 17, 2023

View reviewed changes

pycbc/live/snr_optimizer.py Outdated Show resolved Hide resolved

Bug with d_e options

b36fa69

GarethCabournDavies reviewed Jul 18, 2023

View reviewed changes

pycbc/live/snr_optimizer.py Outdated Show resolved Hide resolved

Adding optimizer-seed

cb35c5d

GarethCabournDavies reviewed Jul 18, 2023

View reviewed changes

examples/live/run.sh Outdated Show resolved Hide resolved

GarethCabournDavies reviewed Jul 18, 2023

View reviewed changes

pycbc/live/snr_optimizer.py Outdated Show resolved Hide resolved

ArthurTolley added 2 commits July 18, 2023 02:14

fixing the d_e optimizer

11b6c67

replacing run.sh code

ba7775e

ArthurTolley requested a review from tdent July 18, 2023 09:32

ArthurTolley and others added 3 commits July 18, 2023 02:34

resolve merge conflict

18e7acd

fixing run.sh

2998884

Merge branch 'master' into live_options

b2c9058

tdent reviewed Jul 18, 2023

View reviewed changes

pycbc/live/snr_optimizer.py Outdated Show resolved Hide resolved

tdent reviewed Jul 18, 2023

View reviewed changes

pycbc/live/snr_optimizer.py Outdated Show resolved Hide resolved

tdent requested changes Jul 18, 2023

View reviewed changes

tdent requested a review from titodalcanton September 11, 2023 14:34

Merge branch 'gwastro:master' into live_options

00672eb

ArthurTolley enabled auto-merge (squash) September 12, 2023 14:59

GarethCabournDavies reviewed Sep 12, 2023

View reviewed changes

bin/pycbc_optimize_snr Show resolved Hide resolved

pycbc/live/snr_optimizer.py Show resolved Hide resolved

ArthurTolley disabled auto-merge September 12, 2023 15:01

tdent reviewed Sep 12, 2023

View reviewed changes

examples/live/run.sh Show resolved Hide resolved

minor CC points

a08c011

remove bug introduced when making CC happier

ba85614

ArthurTolley enabled auto-merge (squash) September 12, 2023 16:48

ArthurTolley merged commit 5542985 into gwastro:master Sep 12, 2023

ArthurTolley deleted the live_options branch September 14, 2023 11:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SNR optimisation options for pycbc_live #4432

SNR optimisation options for pycbc_live #4432

ArthurTolley commented Jul 11, 2023

ArthurTolley commented Jul 18, 2023

ArthurTolley commented Jul 18, 2023

GarethCabournDavies left a comment

GarethCabournDavies commented Jul 18, 2023

ArthurTolley commented Jul 18, 2023

tdent left a comment

GarethCabournDavies commented Sep 11, 2023

ArthurTolley commented Sep 11, 2023

tdent commented Sep 12, 2023

ArthurTolley commented Sep 12, 2023

spxiwh commented Sep 12, 2023

spxiwh commented Sep 12, 2023

ArthurTolley commented Sep 12, 2023

tdent commented Sep 12, 2023

tdent commented Sep 12, 2023

GarethCabournDavies commented Sep 12, 2023

tdent commented Sep 12, 2023

ArthurTolley commented Sep 12, 2023

SNR optimisation options for pycbc_live #4432

SNR optimisation options for pycbc_live #4432

Conversation

ArthurTolley commented Jul 11, 2023

ArthurTolley commented Jul 18, 2023

ArthurTolley commented Jul 18, 2023

GarethCabournDavies left a comment

Choose a reason for hiding this comment

GarethCabournDavies commented Jul 18, 2023

ArthurTolley commented Jul 18, 2023

tdent left a comment

Choose a reason for hiding this comment

GarethCabournDavies commented Sep 11, 2023

ArthurTolley commented Sep 11, 2023

tdent commented Sep 12, 2023

ArthurTolley commented Sep 12, 2023

spxiwh commented Sep 12, 2023

spxiwh commented Sep 12, 2023

ArthurTolley commented Sep 12, 2023

tdent commented Sep 12, 2023

tdent commented Sep 12, 2023

GarethCabournDavies commented Sep 12, 2023

tdent commented Sep 12, 2023

ArthurTolley commented Sep 12, 2023