added scaling of initial pop in snr_optimizer #4561

PRAVEEN-mnl · 2023-11-03T16:04:08Z

We added the init population based on actual parameter values and randomized within bounds earlier it was 0-1 random variables. #4555

pycbc/live/snr_optimizer.py

tdent · 2023-11-03T19:18:10Z

Basically, I think it should be possible to reuse the PSO population init code here rather than writing something separate and more complicated for di.

Also, I think we would like to have debug statements as I noted in the issue #4555 (ie logging.debug) for the parameter values and SNRs being calculated during the optimization.

pycbc/live/snr_optimizer.py

tdent · 2023-11-08T19:10:26Z

See some comments/ suggestions (some of which should deal with codeclimate issues).
The use of logging.debug will not work at the moment but with #4564 it should be possible to make it work, so we may as well use it now.

Has this change been tested to work with both di and PSO optimizers?

ArthurTolley · 2023-11-09T16:02:10Z

Hi @PRAVEEN-mnl, I can't review this until the description has been filled. Thanks

PRAVEEN-mnl · 2023-11-10T09:53:27Z

Yes, the change has been tested to work with both di and PSO optimizers.

tdent · 2023-11-10T16:37:45Z

@ArthurTolley is there anything else you need on this atm?

ArthurTolley · 2023-11-10T16:41:03Z

I will do the review most likely on Monday, if I need anything else then I will let you know 😁

With regards to the description, it is still a bit sparse. Is there a reason why this change was needed? I've seen talk about some snr optimizer bugs but not sure if this is related + how this fixes it. That is the kind of information I would've liked to see in the description @PRAVEEN-mnl.

tdent · 2023-11-10T16:51:05Z

@ArthurTolley evidence of a SNR optimizer bug using di is in the presentations on MDC results from a couple of PyCBC telecons ago. It doesn't make much sense to link LVK internal material here though IMO.

ArthurTolley

All looks good to me, code comments are clean and the change to actual bound values for the random points in the population makes sense.

As long as this has been fully tested with both differential evolution and particle swarm optimization then I am happy with the changes.

(also, sorry for the delay in reviewing this)

titodalcanton · 2023-11-14T17:27:34Z

Reminder to please add the "low latency" label to PyCBC Live PRs, otherwise they will be effectively forgotten.

* added scaling of initial pop * init popn in optimize_di & pso func * added changes in optimize_pso * usig logging.debug for snr

…wastro#4393)" This reverts commit 460d2a40c4bdaf1446fd541f9e7666b24c625d35. The revert is to avoid a bug in the SNR optimizer introduced by this commit and later fixed properly in gwastro#4561.

* added scaling of initial pop * init popn in optimize_di & pso func * added changes in optimize_pso * usig logging.debug for snr

@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 <gareth.cabourndavies@ligo.org> * 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 <gareth.cabourndavies@ligo.org> Co-authored-by: Thomas Dent <thomas.dent@usc.es> * 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 <gareth.cabourndavies@ligo.org> * Comment from Gareth --------- Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org> * [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 <thomas.dent@usc.es> * removing refchecks * Adding option veification for psd variation * Apply suggestions from code review Co-authored-by: Thomas Dent <thomas.dent@usc.es> * 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 <thomas.dent@usc.es> * 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 <gareth.cabourndavies@ligo.org> --------- Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org> --------- Co-authored-by: Ian Harry <ian.harry@ligo.org> Co-authored-by: Arthur Tolley <32394213+ArthurTolley@users.noreply.github.com> Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org> Co-authored-by: Thomas Dent <thomas.dent@usc.es> Co-authored-by: Praveen Kumar <86048588+PRAVEEN-mnl@users.noreply.github.com>

* added scaling of initial pop * init popn in optimize_di & pso func * added changes in optimize_pso * usig logging.debug for snr

added scaling of initial pop

6988b45

PRAVEEN-mnl changed the title ~~added scaling of initial pop~~ added scaling of initial pop in snr_optimizer Nov 3, 2023

tdent reviewed Nov 3, 2023

View reviewed changes