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

Changing pycv into a proper python package #1134

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

Iximiel
Copy link
Member

@Iximiel Iximiel commented Oct 14, 2024

Description

This should solve #1133, the TL;DR is "calling pycv from the python implementation gives problems"

I rewrote the building part of pyCV to look like a python package.

To install it you need plumed available in your path and you just need to use

pip install .

in the pycv dir.

To use it, it is slightly less straightforward (these solution do not work if you installed with pip install -e .):

calling plumed from the shell

When you install with pip you get an extra pycv executable in the bin of the environment. Calling it will print on the shell the path of the .so that contains the pycv actions.
You can copy-paste that path to your plumed.dat and you are ready to go.

calling plumed from python

If you are using plumed in a python script you'll need to have pycv available in the python environment. Then you can get the pycv object to load with:

from pycv import getPythonCVInterface
from plumed import Plumed
...
plmd = Plumed()
...
plmd.cmd("readInputLine",f"LOAD FILE={getPythonCVInterface()}")
...

I need to

  • refine this to compile 100% correctly (aka write a "findPLUMED.cmake")
  • find if there is a more robust way of passing the path
  • update the readme
  • rewrite its part of the CI (both for the plumed-that-calls-python tests and for the python-calling-plumed-that-calls-python tests)
  • update/add new tests
  • have some feedback on the user-friendliness
  • have some feedback on the names

A good number of important projects use scikit-build-core, among them LAMMPS and Cmake, so it is a project destined to remain.


This will unlikely pass the CI for the first few commits

@GiovanniBussi @tonigi, what do you think?

Target release

I would like my code to appear in release 2.10 or 2.11?

Type of contribution
  • changes to code or doc authored by PLUMED developers, or additions of code in the core or within the default modules
  • changes to a module not authored by you
  • new module contribution or edit of a module authored by you
Copyright
  • I agree to transfer the copyright of the code I have written to the PLUMED developers or to the author of the code I am modifying.
  • the module I added or modified contains a COPYRIGHT file with the correct license information. Code should be released under an open source license. I also used the command cd src && ./header.sh mymodulename in order to make sure the headers of the module are correct.
Tests
  • I added a new regtest or modified an existing regtest to validate my changes.
  • I verified that all regtests are passed successfully on GitHub Actions.

plugins/pycv/pythontests/mypycv.py Fixed Show fixed Hide fixed
plugins/pycv/pythontests/test_run.py Fixed Show fixed Hide fixed
plugins/pycv/pythontests/test_run.py Fixed Show fixed Hide fixed
plugins/pycv/src/pycv/__init__.py Fixed Show fixed Hide fixed
@Iximiel Iximiel changed the title Changing pycv into a package Changing pycv into a proper python package Oct 14, 2024
@Iximiel
Copy link
Member Author

Iximiel commented Oct 23, 2024

It works, but I do not understand and I cannot reproduce locally the segmentation faults that are present in the fedora CI.

@Iximiel Iximiel marked this pull request as ready for review October 23, 2024 09:04
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.

1 participant