Resolve "Check for duplicate parameters"

Closes #257 (closed)

added 1 commit

• 1baa944a - First shot at checking if the defined set of parameters does not make any sense

Compare with previous version

@eric.thrane @cjhaster @gregory.ashton

I wrote a little snippet of what this could roughly look like. I have not tested this yet, though.

You may want to use some of the currently available code for this.

https://git.ligo.org/lscsoft/bilby/blob/master/bilby/gw/prior.py#L89

@colm.talbot I tried and did some debugging. I think BBHPriorDict.test_redundancy is not working as intended at the moment. See the code snippet underneath for the problems I spotted:

from bilby.gw.prior import BBHPriorDict
pd = BBHPriorDict(filename='GW150914.prior')
print(pd.test_redundancy('mass_1'))
>>> False
pd.pop('mass_1')
print(pd.test_redundancy('mass_1'))
>>> True
print(pd.test_redundancy('total_mass'))
>>> True

I would expect the opposite here

I will look a bit into this method. I pushed some changes that make the sampler use test_redundancy

added 2 commits

• ead93fa2 - Fixed a bug and moved the declaration of the `redundant` variable
• 16a0a9af - Using `test_redundancy` now for checking for redundant sampling parameters

Compare with previous version

added 91 commits

• 16a0a9af...d9c1d956 - 87 commits from branch master
• dd84fb6d - Merge remote-tracking branch 'origin/master' into 257-check-for-duplicate-parameters
• 210ee8ed - Fixed test_redundancy and wrote comprehensive tests
• 1bf4a2d9 - Added functionality for checking if the prior dict has redundant keys
• 4b3ef997 - Simplified some of the logic

Compare with previous version

unmarked as a Work In Progress

added 1 commit

• a69ac318 - Bowing to the gods of PEP8

Compare with previous version

changed milestone to %0.3.6

added 1 commit

• 0ad8dfaf - Using latest docker image

Compare with previous version

added 1 commit

• 95967660 - Reverted last change

Compare with previous version

Is it possible to print the redundant parameters to screen as part of the error? It would make debugging much simpler.

@colm.talbot If you add three mass priors, you will get a log that looks like this:

12:08 bilby WARNING : mass_1 is a redundant key in this BBHPriorDict.
12:08 bilby WARNING : mass_2 is a redundant key in this BBHPriorDict.
12:08 bilby WARNING : total_mass is a redundant key in this BBHPriorDict.
Traceback (most recent call last):
  File "/home/moritz/Dropbox/PhD/PythonProjects/bilby/examples/injection_examples/fast_tutorial.py", line 84, in <module>
    injection_parameters=injection_parameters, outdir=outdir, label=label)
  File "/home/moritz/Dropbox/PhD/PythonProjects/bilby/bilby/core/sampler/__init__.py", line 137, in run_sampler
    **kwargs)
  File "/home/moritz/Dropbox/PhD/PythonProjects/bilby/bilby/core/sampler/dynesty.py", line 93, in __init__
    **kwargs)
  File "/home/moritz/Dropbox/PhD/PythonProjects/bilby/bilby/core/sampler/base_sampler.py", line 108, in __init__
    self._verify_parameters()
  File "/home/moritz/Dropbox/PhD/PythonProjects/bilby/bilby/core/sampler/base_sampler.py", line 246, in _verify_parameters
    raise IllegalSamplingSetError("Your sampling set contains redundant parameters.")
bilby.core.sampler.base_sampler.IllegalSamplingSetError: Your sampling set contains redundant parameters.

Process finished with exit code 1

Since it is ambiguous as to which key is actually the redundant one, I went for this option. Maybe I can improve this a bit.

added 4 commits

• 85d55800 - Added recent changes to the Changelog
• 1a782754 - Added tests for the BNSPriorDict
• 1eb962e0 - Improved redundancy tests
• 76c5f9df - Added

Compare with previous version

added 1 commit

• 780a0f85 - Fixed a bug

Compare with previous version

I didn't look at anything above the traceback and I'm not sure how uncommon that will be. I think for now we can let it slide and see if more people have this issue.

changed milestone to %0.3.7

added 1 commit

• 50dc14b5 - Fixed a python 2 specific test error

Compare with previous version

added 7 commits

• 50dc14b5...e09ad0d4 - 5 commits from branch master
• a396ddcd - Merge remote-tracking branch 'origin/master' into 257-check-for-duplicate-parameters
• 6f20166e - Fixed some recent Flake 8 issues

Compare with previous version

added 66 commits

• 6f20166e...d9d9d5b1 - 65 commits from branch master
• 05747757 - Merge remote-tracking branch 'origin/master' into 257-check-for-duplicate-parameters

Compare with previous version

changed milestone to %0.4.0

added 10 commits

• 05747757...97937d15 - 9 commits from branch master
• 9260d78d - Merge remote-tracking branch 'origin/master' into 257-check-for-duplicate-parameters

Compare with previous version

added 6 commits

• 9260d78d...bc738186 - 5 commits from branch master
• 382c1a62 - Merge remote-tracking branch 'origin/master' into 257-check-for-duplicate-parameters

Compare with previous version

changed milestone to %0.4.1

approved this merge request

approved this merge request

mentioned in commit e7e24774

merged