Added code to reverse transforms for more flexibility

[ThibeauWouters]

New attempt at improving the flexibility and use of "pre-made" transforms such as masses transforms etc.

Source code changes

I have added two utility functions:

• One function to create a BijectiveTransform given a name_mapping and the forward and backward transformation functions which take arrays as input. That way, we can easily create these "pre-made" transforms by specifying these in a one-liner. At the same time, it allows users to create similar transforms themselves very easily, which therefore also makes the name_mapping no longer hardcoded for the masses since users can change it if they desire, as @thomasckng mentioned some time ago.
• Another function reverses a BijectiveTransform object, as @kazewong suggested in my previous PR attempt.

These functions are then used to simplify the "pre-made" transforms in single_event/transforms.py to be shorter and clearer.

Test changes

• Made sure the GW150914_D.py file in test/integration matches this new implementation.
• Added a new toy example test file to play around with the transforms in a simplified setting.

[thomasckng]

@ThibeauWouters Thanks for the PR! The changes look good!

For the tests, I think the test scripts in test/ are meant to be light-weighted, as they will run whenever there is a new commit. They meant to check if everything is working instead of generating meaningful result. A example with optimized parameters are meant to be put in example/. I am canceling the test runs now.

[ThibeauWouters]

@thomasckng Thanks! I had local copies of the test file to try more agressive hyperparameters but indeed I seemed to have messed them up with the ones in the test directory. Thanks for canceling the runs.

[kazewong]

Test are supposed to be light weight. Reminder to revert these numbers

[kazewong]

I think this is not a good practice in general. Here transform_func_array and inverse_transform_func_array can be potentially harmful functions, and in the long run, this flexibility will create trouble in reproducibility. Any bijective transform should be explicitly declared with the class interface.

[ThibeauWouters]

@kazewong I adapted the code to adapt to your previous comment. But now it feels a bit off in the sense that the "predefined" transforms are classes, that have to be instantiated in order to get the actual transform to be used in PE, and the reverse transforms that are created from the reverse function are objects/instances already. The former require users to give name_mapping, which as I mentioned earlier on is a bit redundant since we would like the name mapping to be fixed for these classes, whereas the former doesn't. So the predefined transform and its reverse are not treated equally in the current implementation.

[kazewong]

@kazewong I adapted the code to adapt to your previous comment. But now it feels a bit off in the sense that the "predefined" transforms are classes, that have to be instantiated in order to get the actual transform to be used in PE, and the reverse transforms that are created from the reverse function are objects/instances already. The former require users to give name_mapping, which as I mentioned earlier on is a bit redundant since we would like the name mapping to be fixed for these classes, whereas the former doesn't. So the predefined transform and its reverse are not treated equally in the current implementation.

There are a couple of things you mentioned here:

1. Re: predefined transform and its reverse are not treated equally: I don't think they should. Inverting a transform target a bijective transform, and predefined transforms are a subset of bijective transforms. For example, reverse_bijective_transform(ScaleTransform((['x'], ['y']), 1.0)) should return a valid transform. As long as this is the interface the user will use, I think this is legit.
2. If we later establish predefined transform can be called with PredefinedTransform(), the interface makes sense to me still, which will be replacing PredefinedTransform() with reverse_bijective_transform(PredefinedTransform()) at the same spot.

I think this is more ergonomic for the user, instead of needing to use a function to call the predefined transform.

Going forward, I do think predefined transforms that enforce the naming strictly should mostly be constructed using the interface PredefinedTransform(). Only transform that does not enforce naming such as ScaleTransform should allow inputs other than the transform parameters.

[ThibeauWouters]

@thomasckng @kazewong I have updated the PR a bit after the discussion yesterday. Let me know if this looks OK to you or if further changes are required.

[ThibeauWouters]

@thomasckng The files have been deleted

[kazewong]

Do you want to further consolidate it? From what I see, this can be just

def named_transform_func...
def named_transform_inverse_func...
ComponentMassesToChirpMassMassRatioTransform =  BijectiveTransform(name_mapping = (["m_1", "m_2"], ["M_c", "q"]),  transform_func =  named_transform, self.inverse_transform_func = named_inverse_transform)

[kazewong]

Please resolve the comment related to further consolidation. But after that it should be good for merging

[ThibeauWouters]

@kazewong I have updated the source code as requested -- is this what you were referring to?