-
Notifications
You must be signed in to change notification settings - Fork 357
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
Add a possibility for defaultvalues sections in config files #4385
Add a possibility for defaultvalues sections in config files #4385
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly happy with this, thanks! Some minor tweaks requested, and I would like to see it work with those tweaks in place.
examples/search/analysis.ini
Outdated
cluster-window = ${statmap|cluster-window} | ||
loudest-keep-values = 15.0:9999999999999 | ||
timeslide-interval = ${coinc-defaultvalues|timeslide-interval} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, as a defaultvalue, why would I be specifying it here? I thought we should get the defaultvalue by default??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or just another illustration?? Perhaps a confusing one without comments if so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is something I'm not sure should be fixed, which is that the interpolation is done before the default values are set, so in order to use this feature, the -defaultvalues
subsection must be specified
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why would you do this. This section should get the defaultvalue anyway. Wouldn't the code work fine with this line just removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this specific case, it is an illustration, and so is not needed (and I am adding a comment to say that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still concerned about this. I don't see any sensible use case for copying an option from -defaultvalues
into another subsection within the same section. It may make sense to copy from say coinc-defaultvalues
into something like plotcoinc
, but why coinc-defaultvalues
into coinc-subsection1
?? Having it here indicates that it has some sane use case.
If there is no sane use case, I'd rather that copying coinc-defaultvalues
into coinc-subsection1
raise a failure, but would be happy if it isn't "advertised" here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I also don't see the indicated comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will change this, I couldnt see a better example before, but found one for fit_by_template
and plot_binnedhist
pycbc/workflow/core.py
Outdated
@@ -380,7 +409,8 @@ def add_ini_opts(self, cp, sec): | |||
# stuff later. | |||
self.unresolved_td_options[opt] = value | |||
else: | |||
self.common_options += [opt, value] | |||
# This option comes from the config file(s) | |||
self.add_or_replace_common_options(opt, value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the change here. I don't think I should be allowing to replace options here, and I don't know why a similar change for the other two cases above wouldn't also be needed? I think if we enforce that:
[coinc]
option1 = 10
[coinc-defaultvalues]
option1=15
is not allowed, then is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be needed in the case of
[coinc-defaultvalues]
option1 = 10
[coinc-subsec]
option1 = 15
As the defaultvalues would be set first, and then the subsec would be overwriting it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right there should be the same for the above examples though - that was missed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fine:
[coinc-defaultvalues]
option1 = 10
[coinc-subsec]
option1 = 15
This should not be:
[coinc]
option1 = 10
[coinc-defaultvalues]
option1=15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But for the bigger question of does this need changing:
If this line needs a change, the other two cases (time-dependent options and File options) might need changes as well. I think the time-dependent option is fine, whereas the File option probably will not be.
Let's look closer at where this section is used, in particular it is only called from here:
Line 571 in 158e038
self.add_ini_opts(self.cp, sec) |
(perhaps this should be _add_ini_opts to avoid anyone trying to call it from anywhere else, which I think would break things).
Now this will be called on all sections and subsections (not defaultvalues explicitly), and you then call _defaultvalues
again for all of these .... This does open up the possibility of [section-tag1-defaultvalues]
and [section-tag2-defaultvalues]
which would conflict and we probably want to avoid.
You are also assuming that [section]
is always sent to add_ini_opts
before any subsections. It is, but if anyone changes that this will break in 'interesting' ways.
I would try and implement a model where you use a new dict defined here:
Line 568 in 158e038
self.common_input_files = [] |
which would keep track of all options added (not values). Then I would explicitly call add_ini_opts
after this for loop:
Line 575 in 158e038
logging.warn(warn_string) |
with self.name + "-defaultvalues"
. This would also have a new keyword argument for "don't override existing values". The default behaviour would be that we fail if an option is already in the new dict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the method to add defaultvalues if and only if the option has not already been added. I don't think there are any action='append'
-style options added via config which could be problematic (I don't think they would have worked before anyway).
This does open up the possibility of
[section-tag1-defaultvalues]
and[section-tag2-defaultvalues]
which would conflict and we probably want to avoid.
I have added a check this so that the -defaultvalues
is only allowed for top-level sections.
Let's look closer at where this section is used, in particular it is only called from here:
Line 571 in 158e038
self.add_ini_opts(self.cp, sec) (perhaps this should be _add_ini_opts to avoid anyone trying to call it from anywhere else, which I think would break things).
add_ini_opts
is also called by the banksim/faithsim codes, so I will delay the rename for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look more closely later, but banksim and faithsim do not use this add_ini_opts function, they use a different, but same named, function in glue (and need to move over to pegasus).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated the name now then
b30411d
to
7e40131
Compare
pycbc/workflow/core.py
Outdated
sec : string | ||
The section containing options for this job. | ||
""" | ||
for opt in cp.options(sec): | ||
if not ignore_existing and opt in self.all_added_options: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look right! Shouldn't we have:
if opt in self.all_added_options:
if ignore_existing:
continue
else:
raise ValueError("Option %s has already been added" % opt)
self.all_added_options.append(opt)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The continue
feels important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes that is right, as otherwise we will end up adding the option again! Thanks for catching that. I will need to go over my tests to confirm this is being checked as not sure how I missed that
pycbc/workflow/core.py
Outdated
@@ -562,18 +567,24 @@ def update_current_tags(self, tags): | |||
|
|||
# collect the options and profile information | |||
# from the ini file section(s) | |||
self.all_added_options = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this a set not a list. All you're doing is is XX in this_list
and that is waaaaayyyyy faster with sets (a dictionary's keys works the same as a set, but we don't have values here, so don't want a dict).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, happy now. Thanks for dealing with overly pedantic comments.
…#4385) * Add a possibility for defaultvalues sections in config files * do not allow [section] and [section-defaultvalues] options * defaultvalues for top-level sections only * underscore on add_ini_opts
This is erroring on main CI now, the problem looks to be something which I checked in earlier iterations, which is that the |
I found what I think is the source and am testing a fix |
A reminder that the workflow search test must complete before merging workflow-related patches :-) |
I missed that this wasn't a 'required' test - lesson learned! |
It's only required if you're touching search functionality, but it's hard to automate that sentence! |
…#4385) * Add a possibility for defaultvalues sections in config files * do not allow [section] and [section-defaultvalues] options * defaultvalues for top-level sections only * underscore on add_ini_opts
* unexpected events * added temp-volume in cl * added rejection sampling * added indexing of mchirp in fit_param * added compensation factor * added corrections * added more corrections * added random number generator seed * added mchirp instead of log(mchirp) in rejection samp * Use probabilities to compensate downsampled kde * Try to deal with flaky failures when apt-getting (#4384) * Implement pre-cuts in fit_over_multiparam for efficiency (#4374) * Make some efficiency savings in pycbc_fit_sngls_over_multiparam * Missing logging line in once case * Add in progress reporting, in case of silent failure * I thought we needed some leeway, but we don't * TD comments * Simplify comment * Migrate to new gwdatafind API (#4383) * Migrate to new gwdatafind API * CC and one regression fix * One more style change * Don't need these options now * Revert "Don't need these options now" This reverts commit 6f2ed1b. * Add a Fisher distribution for sky locations (#4209) * Update transforms.py Adding new_z_to_euler and rotate_euler from pylal.sphericalutils * Update sky_location.py Added new class FisherDist for drawing from sky locations (in ra, dec) * Update sky_location.py Included help message for fisherDist * Update sky_location.py * Code climate issues * Code climate issues * Code-climate changes * Code climate issues * Code climate issues * Update transforms.py changed lal.PI to numpy.pi in new_z_to_euler * Update transforms.py minor changes * Update sky_location.py updated docstring, did changes to make the code look more simple * Update sky_location.py typos corrected * Update sky_location.py code climate changes * Update sky_location.py * Update sky_location.py Included the definitions rvs_polaz and rvs_radec * Update transforms.py Included the definitions decra2polaz and polaz2decra * Update sky_location.py * Update transforms.py code climate issues fixed * Update sky_location.py code climate issues fixed * Update sky_location.py code climate issues * Update sky_location.py fixed code climate issues * Update sky_location.py fixed code climate issues * Update sky_location.py * Update pycbc/distributions/sky_location.py Co-authored-by: Tito Dal Canton <tito@dalcanton.it> * Update sky_location.py * Update sky_location.py * Update transforms.py modified polaz2decra to polaz2radec * Update sky_location.py * Update sky_location.py Forgotten polaz2decra --> polaz2radec * Update sky_location.py Code climate fixes * Update sky_location.py Updated the code for checking the effectiveness of running from config file * Create sky_location_old.py * modified to include from config * Modified to run from config file * Added Fisher distribution to the lista of available distributions * Delete sky_location_old.py * Update sky_location.py * Renamed Fisher to FisherSky * Updated the name to FisherSky * changed Fisher to FisherSky in __all__ * removed decra2polaz and polaz2radec to conversions * Included def decra2polaz, polaz2radec in line 859 * Added angle_unit option * removed decra2polaz and polaz2decra * made carresction * made corrections * Corrected the typing error while removing recra2polaz * Modified angle_unit and updated init file * Empty line on 852 retrieved * corrected * Empty line on852 retrieved * Fixed code climate issues * Sigma conversion taken care of * Fixed code climate issues * Update sky_location.py Codeclimate * Update test_distributions.py Adding fisher_sky to the list of distributions that do not undergo the distribution unittests. * Update sky_location.py Syntax fixed. * Update sky_location.py complying to codeclimate * Refactor using scipy's Rotation * Improve docstring and make warning stricter * Update pycbc/distributions/sky_location.py Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> * Update pycbc/distributions/sky_location.py right angle to correct angle Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> * Update sky_location.py logging issue solved --------- Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito@dalcanton.it> Co-authored-by: P Prasia <prasia.p@ldas-pcdev2.gw.iucaa.in> Co-authored-by: Tito Dal Canton <tito.canton@ligo.org> * add in several missing marginalization combinations (#4376) * allow polarization marginalization with earth rotation * add in missing marg combos * fixes * fixes * fixes * cleanup * cc * cc * Try to fix CI tests (#4394) * Switch FFTW/MKL order of preference * Swap backend order and force gomp import * FIx typo * Do need openmpi in tox.ini * Fix bug in MKL error function * Force FFTW on this one. * CC * Requested Josh changes * Add bank corner plot (#4339) * Add corner plotting script for template banks * Comment where premerger duration has been subtracted from template duration in bank_conversions * add premerger_duration to bank_conversions.py * Move premerger duration into bank_conversions * Use bank's approximant * Dont plot signular values as default * CC complaints * docstring * TDC comments * Keep approximant kwarg as default, TDC comments 2 * Added an OSError exception to MKL check (#4390) * Add a possibility for defaultvalues sections in config files (#4385) * Add a possibility for defaultvalues sections in config files * do not allow [section] and [section-defaultvalues] options * defaultvalues for top-level sections only * underscore on add_ini_opts * Remove default sections from get_subsections (#4399) * Update setup-python action to v4 (#4400) * Replace LOSC with GWOSC (#4401) * Replace LOSC with GWOSC * Fix leftover Python 2-ism * Try to improve codeclimate * Try to fix codeclimate * Try to fix codeclimate * Try to fix codeclimate * Exclude IFO sections when looping through the get_subsections (#4402) * Exclude IFO sections when looping through the get_subsections for plots * Add an example whihc would have previously failed into the search config * Offline singles in workflow (#4386) * Add single-detector options into workflow * Fix a couple of isses which make results pages weird * fix * fix gen.sh to use correct workflow generator * revert change made in error * A couple of points which state coincident when it may not be --------- Co-authored-by: Thomas Dent <thomas.dent@usc.es> Co-authored-by: Ian Harry <ian.harry@ligo.org> Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org> Co-authored-by: Prasia-Pankunni <116145091+Prasia-Pankunni@users.noreply.github.com> Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito@dalcanton.it> Co-authored-by: P Prasia <prasia.p@ldas-pcdev2.gw.iucaa.in> Co-authored-by: Tito Dal Canton <tito.canton@ligo.org> Co-authored-by: Alex Nitz <alex.nitz@gmail.com> Co-authored-by: Arthur Tolley <32394213+ArthurTolley@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito.dalcanton@ijclab.in2p3.fr>
* Try to fix CI tests (#4394) * Switch FFTW/MKL order of preference * Swap backend order and force gomp import * FIx typo * Do need openmpi in tox.ini * Fix bug in MKL error function * Force FFTW on this one. * CC * Requested Josh changes * Add bank corner plot (#4339) * Add corner plotting script for template banks * Comment where premerger duration has been subtracted from template duration in bank_conversions * add premerger_duration to bank_conversions.py * Move premerger duration into bank_conversions * Use bank's approximant * Dont plot signular values as default * CC complaints * docstring * TDC comments * Keep approximant kwarg as default, TDC comments 2 * Added an OSError exception to MKL check (#4390) * Add a possibility for defaultvalues sections in config files (#4385) * Add a possibility for defaultvalues sections in config files * do not allow [section] and [section-defaultvalues] options * defaultvalues for top-level sections only * underscore on add_ini_opts * Remove default sections from get_subsections (#4399) * Update setup-python action to v4 (#4400) * Replace LOSC with GWOSC (#4401) * Replace LOSC with GWOSC * Fix leftover Python 2-ism * Try to improve codeclimate * Try to fix codeclimate * Try to fix codeclimate * Try to fix codeclimate * Exclude IFO sections when looping through the get_subsections (#4402) * Exclude IFO sections when looping through the get_subsections for plots * Add an example whihc would have previously failed into the search config * Offline singles in workflow (#4386) * Add single-detector options into workflow * Fix a couple of isses which make results pages weird * fix * fix gen.sh to use correct workflow generator * revert change made in error * A couple of points which state coincident when it may not be * Rejection samp (#4379) * unexpected events * added temp-volume in cl * added rejection sampling * added indexing of mchirp in fit_param * added compensation factor * added corrections * added more corrections * added random number generator seed * added mchirp instead of log(mchirp) in rejection samp * Use probabilities to compensate downsampled kde * Try to deal with flaky failures when apt-getting (#4384) * Implement pre-cuts in fit_over_multiparam for efficiency (#4374) * Make some efficiency savings in pycbc_fit_sngls_over_multiparam * Missing logging line in once case * Add in progress reporting, in case of silent failure * I thought we needed some leeway, but we don't * TD comments * Simplify comment * Migrate to new gwdatafind API (#4383) * Migrate to new gwdatafind API * CC and one regression fix * One more style change * Don't need these options now * Revert "Don't need these options now" This reverts commit 6f2ed1b. * Add a Fisher distribution for sky locations (#4209) * Update transforms.py Adding new_z_to_euler and rotate_euler from pylal.sphericalutils * Update sky_location.py Added new class FisherDist for drawing from sky locations (in ra, dec) * Update sky_location.py Included help message for fisherDist * Update sky_location.py * Code climate issues * Code climate issues * Code-climate changes * Code climate issues * Code climate issues * Update transforms.py changed lal.PI to numpy.pi in new_z_to_euler * Update transforms.py minor changes * Update sky_location.py updated docstring, did changes to make the code look more simple * Update sky_location.py typos corrected * Update sky_location.py code climate changes * Update sky_location.py * Update sky_location.py Included the definitions rvs_polaz and rvs_radec * Update transforms.py Included the definitions decra2polaz and polaz2decra * Update sky_location.py * Update transforms.py code climate issues fixed * Update sky_location.py code climate issues fixed * Update sky_location.py code climate issues * Update sky_location.py fixed code climate issues * Update sky_location.py fixed code climate issues * Update sky_location.py * Update pycbc/distributions/sky_location.py Co-authored-by: Tito Dal Canton <tito@dalcanton.it> * Update sky_location.py * Update sky_location.py * Update transforms.py modified polaz2decra to polaz2radec * Update sky_location.py * Update sky_location.py Forgotten polaz2decra --> polaz2radec * Update sky_location.py Code climate fixes * Update sky_location.py Updated the code for checking the effectiveness of running from config file * Create sky_location_old.py * modified to include from config * Modified to run from config file * Added Fisher distribution to the lista of available distributions * Delete sky_location_old.py * Update sky_location.py * Renamed Fisher to FisherSky * Updated the name to FisherSky * changed Fisher to FisherSky in __all__ * removed decra2polaz and polaz2radec to conversions * Included def decra2polaz, polaz2radec in line 859 * Added angle_unit option * removed decra2polaz and polaz2decra * made carresction * made corrections * Corrected the typing error while removing recra2polaz * Modified angle_unit and updated init file * Empty line on 852 retrieved * corrected * Empty line on852 retrieved * Fixed code climate issues * Sigma conversion taken care of * Fixed code climate issues * Update sky_location.py Codeclimate * Update test_distributions.py Adding fisher_sky to the list of distributions that do not undergo the distribution unittests. * Update sky_location.py Syntax fixed. * Update sky_location.py complying to codeclimate * Refactor using scipy's Rotation * Improve docstring and make warning stricter * Update pycbc/distributions/sky_location.py Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> * Update pycbc/distributions/sky_location.py right angle to correct angle Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> * Update sky_location.py logging issue solved --------- Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito@dalcanton.it> Co-authored-by: P Prasia <prasia.p@ldas-pcdev2.gw.iucaa.in> Co-authored-by: Tito Dal Canton <tito.canton@ligo.org> * add in several missing marginalization combinations (#4376) * allow polarization marginalization with earth rotation * add in missing marg combos * fixes * fixes * fixes * cleanup * cc * cc * Try to fix CI tests (#4394) * Switch FFTW/MKL order of preference * Swap backend order and force gomp import * FIx typo * Do need openmpi in tox.ini * Fix bug in MKL error function * Force FFTW on this one. * CC * Requested Josh changes * Add bank corner plot (#4339) * Add corner plotting script for template banks * Comment where premerger duration has been subtracted from template duration in bank_conversions * add premerger_duration to bank_conversions.py * Move premerger duration into bank_conversions * Use bank's approximant * Dont plot signular values as default * CC complaints * docstring * TDC comments * Keep approximant kwarg as default, TDC comments 2 * Added an OSError exception to MKL check (#4390) * Add a possibility for defaultvalues sections in config files (#4385) * Add a possibility for defaultvalues sections in config files * do not allow [section] and [section-defaultvalues] options * defaultvalues for top-level sections only * underscore on add_ini_opts * Remove default sections from get_subsections (#4399) * Update setup-python action to v4 (#4400) * Replace LOSC with GWOSC (#4401) * Replace LOSC with GWOSC * Fix leftover Python 2-ism * Try to improve codeclimate * Try to fix codeclimate * Try to fix codeclimate * Try to fix codeclimate * Exclude IFO sections when looping through the get_subsections (#4402) * Exclude IFO sections when looping through the get_subsections for plots * Add an example whihc would have previously failed into the search config * Offline singles in workflow (#4386) * Add single-detector options into workflow * Fix a couple of isses which make results pages weird * fix * fix gen.sh to use correct workflow generator * revert change made in error * A couple of points which state coincident when it may not be --------- Co-authored-by: Thomas Dent <thomas.dent@usc.es> Co-authored-by: Ian Harry <ian.harry@ligo.org> Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org> Co-authored-by: Prasia-Pankunni <116145091+Prasia-Pankunni@users.noreply.github.com> Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito@dalcanton.it> Co-authored-by: P Prasia <prasia.p@ldas-pcdev2.gw.iucaa.in> Co-authored-by: Tito Dal Canton <tito.canton@ligo.org> Co-authored-by: Alex Nitz <alex.nitz@gmail.com> Co-authored-by: Arthur Tolley <32394213+ArthurTolley@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito.dalcanton@ijclab.in2p3.fr> * Adding SEOBNRv5Peak key and function in the final frequency cutoff dictionary (#4408) Co-authored-by: KHUN PHUKON <khun.phukon@ligo.org> * Injection kde (#4409) * unexpected events * added temp-volume in cl * added injection kde * significance bug for trigger_fit. A couple of minor fixes (#4407) * Upgrades to pycbc_grb_inj_finder (#4364) * retaining single ifo injection information * Update pycbc_grb_inj_finder Update according to requested changes from pull request review. * Adding more specific comments Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> * Adding more specific comments out_dict changed to "output dictionary" in comment Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> * Remove xlen Removed xlen from cp_dataset_cont and instead calculated the length of xidentifier within the function. * Reverting comment changes * Remove extra comment * Francesco's bug fix Modified to simplify and correct where ifo information was jumbled together * remove trig_dict and comment * Removed extra lines Removed line and datasets = {} * Update to copyright comment and imports Added our names and accounted for older python version. --------- Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> * Simple bugfix (#4413) Co-authored-by: Bhooshan Gadre <bhooshan.gadre@ligo.org> * Move PyCBC venvs to new IGWN CVMFS server (#4412) * First part of moving venv build to igwn * TESTING HACK. REVERT ME BEFORE MERGING!!! * Try this * Testing * Revert "Testing" This reverts commit c0b9ea0. * Add new CVMFS commands * Remove rsync filter which isn't working * Add error wrap abort handler * Update tools/venv_transfer_commands.sh Co-authored-by: Duncan Macleod <duncan.macleod@ligo.org> * Fix Ian's idiocy (attempt to) and HostChecking * Set to abort for one last test * One final publish test * Add a chmod on the root CVMFS directory * Prepare for merging * Fix mistake --------- Co-authored-by: Duncan Macleod <duncan.macleod@ligo.org> * Get FFT options in pycbc_condition_strain (#4404) * pycbc_optimize_snr: Implement mass dependent bounds on spin parameters (#4176) * Applying spin bounds based upon object mass~ * Updated reference to correct paper * Adding spin bounds based on upper bounds of masses * removing unnecessary additions * Changing max mass2 to the right quantities * Creating maxeta + reusing maxm1 * minor cleanup * fixed min / max eta values * fixed min / max eta * Correcting maxm2 equation * more readable/less redundant comment --------- Co-authored-by: Thomas Dent <thomas.dent@usc.es> * Allow SNR optimizer to use candidate point in initial array (#4393) * allow SNR optimizer to use the candidate point in the initial array of test points * update two LISA docs (#4417) * Update injection_smbhb.ini * Update lisa_smbhb_relbin.ini * Update lisa_smbhb_relbin.ini * Update lisa_smbhb_relbin.ini * Update lisa_smbhb_ldc_pe.rst * Allow pycbc_multi_inspiral to search over a sky grid (#4380) * Allow pycbc_multi_inspiral to search over a sky grid * Adding the possibility to give either a sky grid or a sky position * Change the names of the command lines --ra and --dec to --latitude and --longitude to match the changes made in pycbc_multi_inspiral and remove --processing-scheme because it makes the example crash * Compute the antenna pattern only once for each sky position by keeping fp and fc together * Replace the number of detectors from a hardcoded value to a variable * add TDI AE PSD with confusion noise for LISA (#4389) * add TDI AE PSD with confusion noise * Update analytical_space.py * fix cc issue * Update analytical.py * EM brute (#4299) * Space before colons in eos_utils.py help messages * Enable extrapolation in eos_utils.py when NS is out of bounds * Same check on shape in primary_mass and secondary mass * Function to swap masses and spins of object 1 and object2 * Allow user to request for remnant_mass calculations: 1) extrapolation, 2) swapping object 1 and object 2 if mass2 > mass1, 3) treat as a (dark) BBH and NSBH with NS mass greater than a value probided by the user * Renamned swap_companions to ensure_obj1_is_primary and generalized it * codeclimate * codeclimate * Fixed line that was too long * Fixed docstring and using keywords explicitly in all remnant_mass_* calls * Renamed max_ns_mass as ns_bh_mass_boundary and improved help messages of functions * Improved docstings * Comprehensions in pycbc_brute_bank * Complying to naming convention for spin components in spherical coordinates * Update conversions.py Fixing single code climate complaint. * Prevent duplicates of the HLV page_ifar plot on the summary page (#4421) * Prevent duplicates of the HLV page_ifar plot on the summary page * Rename 'coincidences' as appropriate * add high frequency sky location dependent response for long detectors (#4377) * reset commit, add in high freqeuncy response to detector * update * fixes * Make pegasus 5.0.5 work (#4276) * Revert "try pinning pegasus version (#4265)" This reverts commit 398e135. * Set for_planning flag for subworkflow output map * Add version check so we don't break 5.0.3 * Also include 5.0.4 * make ci faster (#4414) * update * move long log into debug level, allow to set debug level * fix workflow * more explicit control of running external scripts for docs setup at same time * fixes * update * try this * try this ... * update * fixes * update * updates * mistake in example search config file (#4425) * mistake in example search config file * add failure if search runs too long * Range_temp kde (#4424) * unexpected events * added temp-volume in cl * setting the range for temp_kde * setting min based on max kde * logging for min template kde ratio --------- Co-authored-by: Thomas Dent <thomas.dent@usc.es> * Pygrb offline workflow (#4288) * Fixed indexing and autochisq argument in pycbc_multi_inspiral * Only Hanford is labelled with a 1: avoiding pycbc_pygrb_plot_snr_timeseries failure. trigs rather than trigs_or_injs as variable name in pycbc_pygrb_plot_coh_ifosnr * Simplified and update pycbc_pygrb_plot_injs_results; adapted pycbc_pygrb_pp_workflow * Forgotten comma * Massive commit that stitches together pycbc_make_offline_grb_workflow and pycbc_pygrb_pp_workflow, by picking up the latter as a SubWorkflow. * Massive commit that stitches together pycbc_make_offline_grb_workflow and pycbc_pygrb_pp_workflow, by picking up the latter as a SubWorkflow. * Code-climate fixes * More code-climate * Import error flagged by code-climate * Forgotten import fixes * Minor cleanup of comments or commented out code lines * Syntax fix * Defining 2 empty file lists * First part of PR 4288 review comments * One more TODO * opt_to_file is now configparser_value_to_file in pycbc/workflow/core.py * Upgraded configparser_value_to_file with attrs keyword argument * Using configparser_value_to_file throughout the PyGRB workflow generator * Removing 2 unused PyGRB functions * Removed LigolwCBCAlignTotalSpinExecutable, LigolwCBCJitterSkylocExecutable, and PycbcDarkVsBrightInjectionsExecutable support in general and removed LalappsInspinjExecutable support from PyGRB * Codeclimate * bank_veto_file variable needs to be a FileList, not a File * Bug with tags for single IFO SNR plots workflow generator * Fixing pycbc_pygrb_plot_coh_ifosnr * Fixing layout of chi-square plots and section label for individual detector SNRs * Codeclimate * Codeclimate * Codeclimate * Removed FIXME for testing purposes * First changes following up on PR review * Second round of changes following up on PR review * Flagging with an underscore grb utility functions not used outside the files they are defined in * Flagging with an underscore grb utility functions not used outside the files they are defined in * Removed _get_antenna_factors from PyGRB utily file * Removed commented out function from pycbc_pygrb_plot_injs_results * Substituted old unused code with a TODO * Adding injection set name as an option in pycbc_pygrb_efficiency * extend --> append for single files * Documenting setup_pygrb_pp_workflow more appropriately * Reverted name of _get_antenna_responses to get_antenna_responses; cleaner handling of bank-veto-bank-file for pycbc_multi_inspiral job setup. * Line shortened * Avoid skymap v1.1.0 (#4429) * only download LISA response file once (#4423) * Update analytical_space.py * Update analytical_space.py * Introduced a new arguement --skymap-only-ifos in pycbc_live (#4346) * Introduced a new arguement --skymap-only-ifos in pycbc_live * Default skymap_only_ifos changed * skymap_only_ifos as an attribute of LiveEventManager * Test skymaps with V1 as skymap_only_ifos * singles with skymap_only_ifos * Removing commented lines etc * A default seed for SNR opt * Poking CI tests I remove an unnecessary FIXME in the comments, but mostly doing this to restart the CI tests, which seem to have not linked up right. --------- Co-authored-by: Souradeep Pal <souradeep.pal@ldas-pcdev4.ligo.caltech.edu> Co-authored-by: Ian Harry <ian.harry@port.ac.uk> * Fix stageout (#4428) * Fix the help message for stageout script * Fix the help message for stageout and copy map scripts --------- Co-authored-by: connor-mcisaac <connor.mcisaac@outlook.com> * more physical rwrap for LISA SMBHB/SOBHB injection (follow-up fix 2) (#4350) * more physical rwrap * fix cc issues * Update waveform.py * Update companion.txt * Update requirements.txt * Update requirements.txt * Update companion.txt * 10 times tau * Update companion.txt * Update requirements.txt * Update setup.py * add check * Update waveform.py * allow user to choose rwarp * fix cc issue * set 0.2 as minimum rwrap * Update waveform.py * Remove most X509/globus links and hooks from PyCBC (#4375) * Remove most X509/globus links and hooks from PyCBC * Need to update this one as well * Don't need these options now * Update docs/workflow/pycbc_make_coinc_search_workflow.rst Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org> --------- Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org> * fftw_fix (#4418) * Allow pycbc inspiral to run with only numpy fft * Remove Unneeded function from npfft.py --------- Co-authored-by: Neeresh Kumar Perla <neereshkumarperla@Neereshs-MacBook-Pro.local> * Add argument verification for singles (#4365) * Add argument verification for singles * Apply suggestions from code review * remove --enable-single-detector-background * Some more option-checking, and making fixed ifar option actually possible! * fix options in example * CC, typo * CC * Move versioning page generation into its own executable (#4431) * Move versioning page generation into its own executable * Allow duplicated executables * 🤦 * CC changes * CC * save_fig_with_metadata cannot be imported from versioning * Don't like spaces in arguments * Ian's comments * Dont pass executables as files * Same type output from get_code_version_numbers * Rename --longitude and --latitude back to --ra and --dec (#4433) * bugfix - allow non-running of singles (#4439) * bugfix - allow non-running of singles * CC * Revrt wrongly-added test chnages * Add IMRPhenomXAS template duration (#4410) * Add IMRPhenomXAS template duration * removing assertion * fixed fomatting for codeclimate * Adding PhenomXAS duration and removing space fixes * removing extra new line --------- Co-authored-by: Bhooshan Gadre <bhooshan.gadre@ligo.org> * Treat CVMFS better (#4440) * dqsegdb2 update (#4442) * dqsegdb2 update * Update requirements-igwn.txt Co-authored-by: Duncan Macleod <duncan.macleod@ligo.org> --------- Co-authored-by: Ian Harry <ian.harry@ligo.org> Co-authored-by: Duncan Macleod <duncan.macleod@ligo.org> --------- Co-authored-by: Ian Harry <ian.harry@ligo.org> Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org> Co-authored-by: Arthur Tolley <32394213+ArthurTolley@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito.dalcanton@ijclab.in2p3.fr> Co-authored-by: Praveen Kumar <86048588+PRAVEEN-mnl@users.noreply.github.com> Co-authored-by: Prasia-Pankunni <116145091+Prasia-Pankunni@users.noreply.github.com> Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito@dalcanton.it> Co-authored-by: P Prasia <prasia.p@ldas-pcdev2.gw.iucaa.in> Co-authored-by: Tito Dal Canton <tito.canton@ligo.org> Co-authored-by: Alex Nitz <alex.nitz@gmail.com> Co-authored-by: Khun Sang Phukon <khunsang@gmail.com> Co-authored-by: KHUN PHUKON <khun.phukon@ligo.org> Co-authored-by: ETVincent <98178306+ETVincent@users.noreply.github.com> Co-authored-by: Bhooshan Uday Varsha Gadre <bhooshanudaygadre@gmail.com> Co-authored-by: Bhooshan Gadre <bhooshan.gadre@ligo.org> Co-authored-by: Duncan Macleod <duncan.macleod@ligo.org> Co-authored-by: Shichao Wu <wushichao@mail.bnu.edu.cn> Co-authored-by: hoangstephanie <105010876+hoangstephanie@users.noreply.github.com> Co-authored-by: SouradeepPal <126229608+SouradeepPal@users.noreply.github.com> Co-authored-by: Souradeep Pal <souradeep.pal@ldas-pcdev4.ligo.caltech.edu> Co-authored-by: Ian Harry <ian.harry@port.ac.uk> Co-authored-by: connor-mcisaac <connor.mcisaac@outlook.com> Co-authored-by: Neeresh Kumar <neeresh@users.noreply.github.com> Co-authored-by: Neeresh Kumar Perla <neereshkumarperla@Neereshs-MacBook-Pro.local>
…#4385) * Add a possibility for defaultvalues sections in config files * do not allow [section] and [section-defaultvalues] options * defaultvalues for top-level sections only * underscore on add_ini_opts
* unexpected events * added temp-volume in cl * added rejection sampling * added indexing of mchirp in fit_param * added compensation factor * added corrections * added more corrections * added random number generator seed * added mchirp instead of log(mchirp) in rejection samp * Use probabilities to compensate downsampled kde * Try to deal with flaky failures when apt-getting (gwastro#4384) * Implement pre-cuts in fit_over_multiparam for efficiency (gwastro#4374) * Make some efficiency savings in pycbc_fit_sngls_over_multiparam * Missing logging line in once case * Add in progress reporting, in case of silent failure * I thought we needed some leeway, but we don't * TD comments * Simplify comment * Migrate to new gwdatafind API (gwastro#4383) * Migrate to new gwdatafind API * CC and one regression fix * One more style change * Don't need these options now * Revert "Don't need these options now" This reverts commit 6f2ed1b. * Add a Fisher distribution for sky locations (gwastro#4209) * Update transforms.py Adding new_z_to_euler and rotate_euler from pylal.sphericalutils * Update sky_location.py Added new class FisherDist for drawing from sky locations (in ra, dec) * Update sky_location.py Included help message for fisherDist * Update sky_location.py * Code climate issues * Code climate issues * Code-climate changes * Code climate issues * Code climate issues * Update transforms.py changed lal.PI to numpy.pi in new_z_to_euler * Update transforms.py minor changes * Update sky_location.py updated docstring, did changes to make the code look more simple * Update sky_location.py typos corrected * Update sky_location.py code climate changes * Update sky_location.py * Update sky_location.py Included the definitions rvs_polaz and rvs_radec * Update transforms.py Included the definitions decra2polaz and polaz2decra * Update sky_location.py * Update transforms.py code climate issues fixed * Update sky_location.py code climate issues fixed * Update sky_location.py code climate issues * Update sky_location.py fixed code climate issues * Update sky_location.py fixed code climate issues * Update sky_location.py * Update pycbc/distributions/sky_location.py Co-authored-by: Tito Dal Canton <tito@dalcanton.it> * Update sky_location.py * Update sky_location.py * Update transforms.py modified polaz2decra to polaz2radec * Update sky_location.py * Update sky_location.py Forgotten polaz2decra --> polaz2radec * Update sky_location.py Code climate fixes * Update sky_location.py Updated the code for checking the effectiveness of running from config file * Create sky_location_old.py * modified to include from config * Modified to run from config file * Added Fisher distribution to the lista of available distributions * Delete sky_location_old.py * Update sky_location.py * Renamed Fisher to FisherSky * Updated the name to FisherSky * changed Fisher to FisherSky in __all__ * removed decra2polaz and polaz2radec to conversions * Included def decra2polaz, polaz2radec in line 859 * Added angle_unit option * removed decra2polaz and polaz2decra * made carresction * made corrections * Corrected the typing error while removing recra2polaz * Modified angle_unit and updated init file * Empty line on 852 retrieved * corrected * Empty line on852 retrieved * Fixed code climate issues * Sigma conversion taken care of * Fixed code climate issues * Update sky_location.py Codeclimate * Update test_distributions.py Adding fisher_sky to the list of distributions that do not undergo the distribution unittests. * Update sky_location.py Syntax fixed. * Update sky_location.py complying to codeclimate * Refactor using scipy's Rotation * Improve docstring and make warning stricter * Update pycbc/distributions/sky_location.py Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> * Update pycbc/distributions/sky_location.py right angle to correct angle Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> * Update sky_location.py logging issue solved --------- Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito@dalcanton.it> Co-authored-by: P Prasia <prasia.p@ldas-pcdev2.gw.iucaa.in> Co-authored-by: Tito Dal Canton <tito.canton@ligo.org> * add in several missing marginalization combinations (gwastro#4376) * allow polarization marginalization with earth rotation * add in missing marg combos * fixes * fixes * fixes * cleanup * cc * cc * Try to fix CI tests (gwastro#4394) * Switch FFTW/MKL order of preference * Swap backend order and force gomp import * FIx typo * Do need openmpi in tox.ini * Fix bug in MKL error function * Force FFTW on this one. * CC * Requested Josh changes * Add bank corner plot (gwastro#4339) * Add corner plotting script for template banks * Comment where premerger duration has been subtracted from template duration in bank_conversions * add premerger_duration to bank_conversions.py * Move premerger duration into bank_conversions * Use bank's approximant * Dont plot signular values as default * CC complaints * docstring * TDC comments * Keep approximant kwarg as default, TDC comments 2 * Added an OSError exception to MKL check (gwastro#4390) * Add a possibility for defaultvalues sections in config files (gwastro#4385) * Add a possibility for defaultvalues sections in config files * do not allow [section] and [section-defaultvalues] options * defaultvalues for top-level sections only * underscore on add_ini_opts * Remove default sections from get_subsections (gwastro#4399) * Update setup-python action to v4 (gwastro#4400) * Replace LOSC with GWOSC (gwastro#4401) * Replace LOSC with GWOSC * Fix leftover Python 2-ism * Try to improve codeclimate * Try to fix codeclimate * Try to fix codeclimate * Try to fix codeclimate * Exclude IFO sections when looping through the get_subsections (gwastro#4402) * Exclude IFO sections when looping through the get_subsections for plots * Add an example whihc would have previously failed into the search config * Offline singles in workflow (gwastro#4386) * Add single-detector options into workflow * Fix a couple of isses which make results pages weird * fix * fix gen.sh to use correct workflow generator * revert change made in error * A couple of points which state coincident when it may not be --------- Co-authored-by: Thomas Dent <thomas.dent@usc.es> Co-authored-by: Ian Harry <ian.harry@ligo.org> Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org> Co-authored-by: Prasia-Pankunni <116145091+Prasia-Pankunni@users.noreply.github.com> Co-authored-by: Francesco Pannarale <pannarale@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito@dalcanton.it> Co-authored-by: P Prasia <prasia.p@ldas-pcdev2.gw.iucaa.in> Co-authored-by: Tito Dal Canton <tito.canton@ligo.org> Co-authored-by: Alex Nitz <alex.nitz@gmail.com> Co-authored-by: Arthur Tolley <32394213+ArthurTolley@users.noreply.github.com> Co-authored-by: Tito Dal Canton <tito.dalcanton@ijclab.in2p3.fr>
After the discussion on #4363, a defaultvalues section seemed more sensible than specifically overriding certain options