gwastro / pycbc

Core package to analyze gravitational-wave data, find signals, and study their parameters. This package was used in the first direct detection of gravitational waves (GW150914), and is used in the ongoing analysis of LIGO/Virgo data.
http://pycbc.org
GNU General Public License v3.0
313 stars 347 forks source link

TypeError: 'cmp' is an invalid keyword argument for sort() #3814

Closed MariaAssiduo closed 2 years ago

MariaAssiduo commented 2 years ago

Hi. I am trying to use banksim with the waveforms IMRPhenomXPHM using as source conda activate igwn-py39-20210916. Inside the banksim configuration file (.ini) I have under [executable]

banksim = /cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py39-20210916/bin/pycbc_banksim

Then, when I run pycbc_make_banksim --config file.ini, I get the following error message:

Traceback (most recent call last): File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py39-20210916/bin/pycbc_splitbank", line 134, in tt = sorted(template_bank_table, cmp=mchirp_sort) TypeError: 'cmp' is an invalid keyword argument for sort()

It seems that in Python3 is necessary to rewrite cmp function as a key function instead? If so, how to overcome this issue in the script pycbc_splitbank in order to use banksim?

Thanks in advance for any suggestion.

MariaAssiduo commented 2 years ago

Hi. I did the following modifications in my pycbc version of pycbc_splitbank script:

....from functools import cmp_to_key ... ... def cmp(a, b): return (a > b) - (a < b)

def mchirp_sort(x, y): mc1, e1 = pycbc.pnutils.mass1_mass2_to_mchirp_eta(x.mass1, x.mass2) mc2, e2 = pycbc.pnutils.mass1_mass2_to_mchirp_eta(y.mass1, y.mass2) return cmp(mc1, mc2)

def frequency_cutoff_sort(x, y): p1 = pycbc.pnutils.frequency_cutoff_from_name(args.sort_frequency_cutoff, x.mass1, x.mass2, x.spin1z, x.spin2z) p2 = pycbc.pnutils.frequency_cutoff_from_name(args.sort_frequency_cutoff, y.mass1, y.mass2, y.spin1z, y.spin2z) return cmp(p1, p2)

tt = template_bank_table

if args.sort_frequency_cutoff: tt = sorted(template_bank_table, key=cmp_to_key(frequency_cutoff_sort))

if args.sort_mchirp: tt = sorted(template_bank_table, key=cmp_to_key(mchirp_sort))

For now my banksim is running without errors about cmp function.

Thanks.

MariaAssiduo commented 2 years ago

cmp_Py3_MA.pdf Here few slides to justify my changes in the code. Any comments and suggestions are welcome. Thanks.

titodalcanton commented 2 years ago

Hi @MariaAssiduo, would you be able to open a pull request with this change?

ahnitz commented 2 years ago

@MariaAssiduo WOW! That's really impressive that you put in the work to put together a presentation. If you know how you want the code changed, we are very happy to accept PRs. PyCBC is community-developed, so we welcome anyone to help make it work better (or in this case fix it!). Thank you so much for looking into this problem.

MariaAssiduo commented 2 years ago

Hi @titodalcanton I am not sure but I can try. Thanks.

MariaAssiduo commented 2 years ago

Hi @ahnitz, thank you very much. What do you mean with PRs? I am not really an expert of Python, and I try to fix the issues just studying free online resources whenever I need it for my analysis..

titodalcanton commented 2 years ago

More information on how to contribute can be found in the file https://github.com/gwastro/pycbc/blob/master/CONTRIBUTING.md