Pull in some functions from catalog_tools

This PR pulls in some parts of a script from catalog_tools. Some of the changes here are:

Split functions out into separate files
Change log function to Log object, to avoid global history
Change uses of open-.close blocks to use context managers
Don't .close files opened by context managers
Don't use file as a variable name; it's a python keyword

I agree with both changes. I think some single res simulations had a stupid choice for comparable simulation, but other than that metadata, I think this bug wasn’t serious. Good catch!

Geoffrey

Sent from my iPhone

On Dec 20, 2019, at 08:24, Mike Boyle notifications@github.com wrote:

@moble commented on this pull request.

In sxs/formats/lvc/metadata.py:

current_mass2 = catalog[key]['initial_mass2']

current_spin1 = catalog[key]['initial_dimensionless_spin1']

current_spin2 = catalog[key]['initial_dimensionless_spin2']

current_mass_ratio = current_mass1 / current_mass2

current_spin1_magnitude = np.linalg.norm(current_spin1)

current_spin2_magnitude = np.linalg.norm(current_spin2)

mass_spin_diff = (

np.abs(mass_ratio - current_mass_ratio)

np.abs(spin1_magnitude - current_spin1_magnitude)

np.abs(spin2_magnitude - current_spin2_magnitude)

)

if mass_spin_diff < mass_spin_diff_best:

mass_spin_diff_best = mass_spin_diff

key_best = key

resolution_best = np.max(catalog_resolutions[key]) @geoffrey4444 I'm almost ready to test this, but I found one issue that I think you need to take a look at.

Presumably key in this line is supposed to be key_best. Unfortunately, the code as is (which is presumably what you've used in the past) will actually run without notifying you of any issues, but the value of key in this line is just the last value from the loop — which I guess may or may not actually be the best. So it seems likely that there are some incorrect choices of resolution in what you've produced with this.

Also, it gives me the heebie-jeebies to define key_best = ""; I'd prefer to define it as key_best = has_multiple_resolutions[0].

Do these two changes make sense to you?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

@geoffrey4444 I've just made a few more changes to the main new function, convert_simulation. They shouldn't affect the output in any way, but the arguments to the function have changed to simplify calling it and reduce the number of files that need to be kept around.

The resolution argument is now optional. If the argument is given, the behavior is the same as before. Otherwise, the resolution is automatically detected from the path (and will raise an exception if that fails). Essentially, it searches the path string for the first match of the regex Lev[-0-9]*, and converts that last part to an int. Note that the negative sign is used because cases with Lev-1 exist.
sxs_catalog_resolutions_path has been removed because the information in that file can be recalculated almost as fast as it can be read. Recalculating will avoid the possibility that this file is stale, and obviate the need to recreate it when the catalog is updated. (The recalculation is done by the new function sxs.zenodo.catalog.resolutions_for_simulations.)
sxs_catalog_metadata_path has been renamed to sxs_catalog_path because the former conflicts with other usage of "metadata" in this package, plus it no longer needs to be disambiguated from sxs_catalog_resolutions_path. This is also now optional. If the argument is given, the behavior is the same as before — unless the file isn't found, in which case it's downloaded automatically from black-holes. By default, a local copy of the catalog is kept in ~/.sxs/catalog/; if not found there, it's downloaded automatically and stored there.

I've also checked that it actually runs and produces output that looks right. Specifically, I ran both this function and your original script on just one system, and only with modes=3. But the resulting h5 files were bitwise-identical, except in auxiliary-info/ConversionLog.txt and auxiliary-info/convert_sxs_to_lvc.py, which differ as expected because (a) the input arguments recorded to the log are different, and (b) because the scripts are different.

The only thing I can think of that might be improved is removing that auxiliary-info/convert_sxs_to_lvc.py dataset, to instead be something holding the output of sxs.__version__ (which is basically the git commit date).

But otherwise, I think this is ready to merge.

Thanks, Mike! This is awesome. Let’s make that last change you suggest and then merge.

Geoffrey

Sent from my iPhone

On Jan 14, 2020, at 13:28, Mike Boyle notifications@github.com wrote:

@geoffrey4444 I've just made a few more changes to the main new function, convert_simulation. They shouldn't affect the output in any way, but the arguments to the function have changed to simplify calling it and reduce the number of files that need to be kept around.

The resolution argument is now optional. If the argument is given, the behavior is the same as before. Otherwise, the resolution is automatically detected from the path (and will raise an exception if that fails). Essentially, it searches the path string for the first match of the regex Lev[-0-9]*, and converts that last part to an int. Note that the negative sign is used because cases with Lev-1 exist. sxs_catalog_resolutions_path has been removed because the information in that file can be recalculated almost as fast as it can be read. Recalculating will avoid the possibility that this file is stale, and obviate the need to recreate it when the catalog is updated. (The recalculation is done by the new function sxs.zenodo.catalog.resolutions_for_simulations.) sxs_catalog_metadata_path has been renamed to sxs_catalog_path because the former conflicts with other usage of "metadata" in this package, plus it no longer needs to be disambiguated from sxs_catalog_resolutions_path. This is also now optional. If the argument is given, the behavior is the same as before — unless the file isn't found, in which case it's downloaded automatically from black-holes. By default, a local copy of the catalog is kept in ~/.sxs/catalog/; if not found there, it's downloaded automatically and stored there. I've also checked that it actually runs and produces output that looks right. Specifically, I ran both this function and your original script on just one system, and only with modes=3. But the resulting h5 files were bitwise-identical, except in auxiliary-info/ConversionLog.txt and auxiliary-info/convert_sxs_to_lvc.py, which differ as expected because (a) the input arguments recorded to the log are different, and (b) because the scripts are different.

The only thing I can think of that might be improved is removing that auxiliary-info/convert_sxs_to_lvc.py dataset, to instead be something holding the output of sxs.version (which is basically the git commit date).

But otherwise, I think this is ready to merge.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

Okay, I've removed the convert_sxs_to_lvc.py dataset and replaced it with CodeVersions.txt, which gives the current versions of relevant packages in standard pip/conda format. For example, right now my output looks like this:

python==3.7.6
numpy==1.18.1
scipy==1.4.1
h5py==2.10.0
# h5py.version.api_version: 1.8
# h5py.version.hdf5_version: 1.10.4
romspline==1.1.4
sxs==2020.1.14.16.23.40

So with that, I'll merge.

sxs-collaboration / sxs

Pull in some functions from catalog_tools #5