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

Missing requirements.txt and venv in some Python-based examples #547

Open
MakisH opened this issue Jul 2, 2024 · 13 comments
Open

Missing requirements.txt and venv in some Python-based examples #547

MakisH opened this issue Jul 2, 2024 · 13 comments

Comments

@MakisH
Copy link
Member

MakisH commented Jul 2, 2024

A generalization of #190.

Everything Nutils-based is already covered. We need to adapt our FEniCS and all Python-only examples.

@BenjaminRodenberg
Copy link
Member

BenjaminRodenberg commented Aug 29, 2024

A list of open ToDos:

@BenjaminRodenberg
Copy link
Member

That's actually quite a long list of cases that need an update. For the pure python / FEniCS cases the procedure is clear. Regarding the FMI cases some input from @uekerman would be appreciated since I'm not that much into the topic.

@BenjaminRodenberg
Copy link
Member

BenjaminRodenberg commented Aug 29, 2024

@MakisH suggested to not put the requirements.txt and venv inside the participant directory but rather into the solver directory. I.e. not structure for the oscillator as given in #556:

oscillator
├── mass-left-python
│   ├── requirements.txt  *
│   ├── run.sh
│   └── .venv  *
├── mass-right-python
│   ├── requirements.txt  *
│   ├── run.sh
│   └── .venv  *
└── solver-python
    └── oscillator.py

Instead we can use:

oscillator
├── mass-left-python
│   └── run.sh
├── mass-right-python
│   └── run.sh
└── solver-python
    ├── requirements.txt
    ├── .venv
    └── oscillator.py

The files/folders marked with a * are duplicates and, therefore, the second solution would save us a lot of implementation and maintainment work.

Note: I think this would also apply to the nutils cases and would justify introducing a solver-nutils directory containing the requirements.txt file.

BenjaminRodenberg added a commit that referenced this issue Aug 29, 2024
…utorial (#556)

* Related to #547 
* Add requirements to oscillator tutorial
* Add requirements to oscillator-overlap tutorial
* Update run.sh to use requirements.txt and create venv

---------

Co-authored-by: Benjamin Rodenberg <[email protected]>
@MakisH
Copy link
Member Author

MakisH commented Aug 29, 2024

Note: I think this would also apply to the nutils cases and would justify introducing a solver-nutils directory containing the requirements.txt file.

Possibly, yes, but let's keep these as independent PRs. Let's focus on getting requirements.txt everywhere first. Whenever we open the box of "what is similar enough to be merged", that will be a long path.

@BenjaminRodenberg
Copy link
Member

BenjaminRodenberg commented Aug 29, 2024

Note: I think this would also apply to the nutils cases and would justify introducing a solver-nutils directory containing the requirements.txt file.

Possibly, yes, but let's keep these as independent PRs. Let's focus on getting requirements.txt everywhere first. Whenever we open the box of "what is similar enough to be merged", that will be a long path.

@MakisH True. #498 is already an exception: The requirements.txt files for the two python participants should be identical, but there is no solver folder and the participants actually have independent scripts. It's just coincidence they have identical dependencies. Let's focus on the first step as you suggested.

Edit: Or are you only referring to the nutils cases with this comment?

@MakisH
Copy link
Member Author

MakisH commented Aug 29, 2024

If there is a common folder (e.g., solver-python), then I would add the venv there. If not, I would not force one just for the dependencies of each script, even if two scripts happen to have the same dependencies at the moment. That would be unnecessary coupling.

But with the "let's not open that box", I was referring to Nutils.

@BenjaminRodenberg
Copy link
Member

If there is a common folder (e.g., solver-python), then I would add the venv there. If not, I would not force one just for the dependencies of each script, even if two scripts happen to have the same dependencies at the moment. That would be unnecessary coupling.

But with the "let's not open that box", I was referring to Nutils.

Sounds good. 👍 Then I think we can start implementing 🎉 Already discussed this with @NiklasVin. He will take care of all the necessary updates.

@uekerman
Copy link
Member

Regarding the FMI cases some input from @uekerman would be appreciated since I'm not that much into the topic.

Only requiring fmiprecice should do the job, no? Everything else is a dependency of this package.

@BenjaminRodenberg
Copy link
Member

For the FEniCS cases I just ran into the the problem that accessing FEniCS from a venv does not work (https://fenicsproject.discourse.group/t/using-fenics-with-python-venv/4887).

Using anaconda and the corresponding FEniCS package (see https://fenicsproject.org/download/archive/) should be possible. We also have the fenics adapter and the python bindings on anaconda (see https://anaconda.org/conda-forge/fenicsprecice). But this is again a completely different approach...

BenjaminRodenberg added a commit to TobiasEppacher/tutorials that referenced this issue Aug 30, 2024
@BenjaminRodenberg
Copy link
Member

Seems like python3 -m venv --system-site-packages ../solver-fenics/.venv does the trick (assuming FEniCS was installed using apt). I think this is a proper solution. See 3a00c01.

@BenjaminRodenberg
Copy link
Member

BenjaminRodenberg commented Sep 5, 2024

I think python3 -m venv ../solver-fenics/.venv might be a bad idea after all. See #567.

@fsimonis
Copy link
Member

fsimonis commented Sep 6, 2024

The easy way out is to keep all solver information in the solver directory including requirements.txt, and use a local venv in the participant directory where the run script is.

pip uses a user-local cache for requests even when using a venv, so this shouldn't be too much of an issue. pip cache dir

The problem with the current layout is that run scripts are pretty ugly.
A pretty straight-forward solution for this is to make solvers into mini packages.

Example for the oscillator:

This needs a subdirectory as it contains multiple files. Single-file packages don't need this extra step.

oscillator/solver-python
├── oscillator
│   ├── oscillator.py
│   ├── problemDefinition.py
│   └── timeSteppers.py
└── pyproject.toml
[project]
name = "oscillator"
version = "0" # we don't care
dependencies = [
    "numpy",
    "pyprecice~=3.0",
    "scipy"
]

# Defining entry points that are available in PATH after installation.
# This is practical for calling them and killing them by name.
[project.scripts]
leftMass = "oscillator.oscillator:runLeft"
rightMass = "oscillator.oscillator:runRight"
from . import problemDefinition
from . import timeSteppers

class Participant(Enum):
    MASS_LEFT = "Mass-Left"
    MASS_RIGHT = "Mass-Right"


def runLeft():
    run(Participant.MASS_LEFT.value)


def runRight():
    run(Participant.MASS_RIGHT.value)


def run(participant_name):
    parser = argparse.ArgumentParser()
    # ...

@BenjaminRodenberg BenjaminRodenberg changed the title Missing requirements.txt and venv in some Python-based examples Missing requirements.txt and venv in some Python-based examples Sep 6, 2024
@BenjaminRodenberg
Copy link
Member

Thanks for the input @fsimonis! Modularizing some of the python-based solvers better would definitely help. I would still suggest to first go for the simple approach and live with a global requirements.txt in the solver directory and create the venv in the participant directory. Creating proper packages for the solvers I see as a second step that we could add later (created an issue #568).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants