search

gwastro / pycbc

Core package to analyze gravitational-wave data, find signals, and study their parameters. This package was used in the first direct detection of gravitational waves (GW150914), and is used in the ongoing analysis of LIGO/Virgo data.
http://pycbc.org
GNU General Public License v3.0
314 stars 351 forks source link

pycbc should directly depend on lalsuite wheel #2367

Open ahnitz opened 6 years ago

ahnitz commented 6 years ago

I've been testing the lalsuite wheels for a while and I think they are working out pretty well. I can run the full PE and coincident workflow from them. As such, I think we should declare an explicit dependency on lalsuite as a python package. As part of this we can clean up the travis to not have to build lalsuite anymore. Versions for releases can be pinned using a requirements-release.txt file that picks out a specific version hash. That will then be the only place the release version is updated, instead of the somewhat convoluted way now.

What do you all think? Any issues that come to mind?

duncan-brown commented 6 years ago

@ahnitz I think that's fine, but should we depend on a specific, known, and working version?

ahnitz commented 6 years ago

In the requirements-release file yes, but only a minimum version otherwise.

duncan-brown commented 6 years ago

Sounds good.

tdent commented 6 years ago

I have nothing to say on this .. (at least not on the timescale where you might want feedback)

spxiwh commented 6 years ago

Would I still be able to run PyCBC against a development version of lalsuite? For example if I want to use PhenomZ or SEOBNRv{\pi}?

duncan-brown commented 6 years ago

@spxiwh yes, you could just pip uninstall the installed version of lalsuite and install another version in your virtualenv or container.

@ahnitz since no one has raised any major objections to this, I'm going to make the fix once https://github.com/pegasus-isi/pegasus/pull/14 is merged as I can then drop support for the bundled executables.

This will simplify both the Docker image and Travis a lot.

We should be able to do this right now, even without changing the lal tag as

pip install lalsuite==6.48.1.dev20180717

gets us

(test-lal)[dbrown@sugwg-condor ~]$ lalapps_version 
LAL: 6.18.0.1-dev20180717 (CLEAN 89a30fcf86f5d23455303e32051a87b0e3c3084a)

which is the current release tag.

ahnitz commented 6 years ago

@duncan-brown Good to know. Not changing the lal tag initially will avoid some headaches.