moble / scri

Python/numba code for manipulating time-dependent functions of spin-weighted spherical harmonics on future null infinity
MIT License
18 stars 20 forks source link

remove_avg_com_motion requires path_to_waveform_h5 option #68

Open markscheel opened 2 years ago

markscheel commented 2 years ago

remove_avg_com_motion makes you provide path_to_waveform_h5 even if you tell it not to read that file (by passing in a waveform object w_m) and even if that file doesn't exist. That actual h5 file doesn't need to exist, but path_to_waveform_h5 must have the form 'something.h5/group' or '/full/path/something.h5/group', which it will always parse and it will throw an error if it cannot parse, because it uses the /full/path and group pieces later on in the code. If you don't provide path_to_horizons_h5 or path_to_matter_h5 arguments, then it will look for Horizons.h5 and Matter.h5 inside '/full/path' (or inside cwd if the given path begins with 'something.h5'). Also, both '/full/path' and 'group' will be used for output of figures and filenames of figure files if plotting is turned on.

Note that if your Horizons.h5 happens to be in cwd, then the default value of path_to_waveform_h5 will work correctly if you don't specify path_to_horizons_h5. (but if you turn plotting on, the plot filename will contain 'Extrapolated_N2' which might be misleading).

It would be less confusing if the options were done differently.

Perhaps path_to_horizons_h5 could be the required option, and then internally the variable directory would come from path_to_horizons_h5. Then path_to_waveform_h5 could be truly optional and have a default of None. The internal variable subdir could have its own option (which the user would be required to specify if path_to_waveform_h5 is None and if plotting is turned on).

markscheel commented 2 years ago

I came across this because I used the new w_m option of remove_avg_com_motion and I thought the 'obvious' thing to do was to pass it path_to_waveform_h5=None since I didn't have a waveform h5 file.