PolyChord / PolyChordLite

Public version of PolyChord: See polychord.co.uk for PolyChordPro
https://polychord.io/
Other
83 stars 26 forks source link

Added Sanity checks, to ensure congruent Settings. #50

Closed appetrosyan closed 2 years ago

appetrosyan commented 4 years ago

Meant to solve #47. May need testing, but instead of catching errors and raising errors early, this produces a warning (for now, it's trivial to change to raise ValueError).

I'm not quite sure about the convention of using negative numbers to set defaults. This is not pythonic. The justification is that this is the convention in other languages, and when interoperating, it's easier to maintain only one version. However, we're already providing the defaults in the doctoring, so we need to duplicate the effort anyway. It may be better to add another module that contains the defaults, and update it.

appetrosyan commented 4 years ago

Ok. I specifically checked if it's python2 compatible (otherwise the Python3 way is using a @dataclass object). Pushing now!

appetrosyan commented 4 years ago

About the shift from *args/**kwargs to explicit keyword arguments. It's not a breaking change. The opposite would be though.

Python dictionaries are unordered by default, meaning that **kwargs can't be interpreted as positional arguments. By introducing the keyword arguments in the signature, we allow them being passed in the order in which they are defined.

So by definition every configuration that used to work with the old way, should work now too.

An alternative (yet also neat way) that preserves the **kwargs style would be to create a dictionary of default values. This will not broaden the scope of acceptable input, but will have the benefits of clarity and conciseness (cobaya does it in a similar way).

I have considered other approaches, e.g. @dataclass with slots, as per here, but that would definitely break python2 compatibility.