Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fj-contrib #143

Merged
merged 6 commits into from
Nov 11, 2022
Merged

fj-contrib #143

merged 6 commits into from
Nov 11, 2022

Conversation

jmduarte
Copy link
Collaborator

@jmduarte jmduarte commented Nov 9, 2022

  • Add fastjet-contrib as a submodule
  • Compile fastjet-contrib as part of setup.pyas a submodule
  • Reuse LundPlane code from fastjet-contrib
  • Check if it runs/passes tests

This approach adds some overhead (compiling all of fastjet-contrib), but in principle allows us to add helper functions to use other contributions like SoftDrop, Nsubjettiness, EnergyCorrelationFunctions, etc.

src/_ext.cpp Outdated Show resolved Hide resolved
@jmduarte
Copy link
Collaborator Author

The error occurs when importing fastjet so I'm not compiling/linking things appropriately. Maybe @chrispap95 or someone else has a clue on how to fix it?

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/opt/python/cp39-cp39/lib/python3.9/site-packages/fastjet/__init__.py", line 7, in <module>
    import fastjet._ext  # noqa: F401, E402
ImportError: /opt/python/cp39-cp39/lib/python3.9/site-packages/fastjet/_ext.cpython-39-x86_64-linux-gnu.so: undefined symbol: _ZTVN7fastjet7contrib13LundGeneratorE

@chrispap95
Copy link
Collaborator

@jmduarte it seems that each contrib is creating a .a lib and these libs are not linked against _ext. Perhaps, you could add these somehow inPybind11Extension(..., library_dirs=[], ... ). The only problem I see is that these plugins generate static libraries and pybind11 seems to look only for shared libs. @henryiii might know how to solve this.

@jmduarte jmduarte requested a review from lgray November 11, 2022 00:52
@lgray
Copy link
Collaborator

lgray commented Nov 11, 2022

We can probably wait to create a new release since the current implementation is correct as far as I can tell. Otherwise this looks fine.

@lgray lgray enabled auto-merge (squash) November 11, 2022 00:57
@lgray lgray merged commit f8e5e57 into scikit-hep:main Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants