molmod / psiflow

scalable molecular simulation
https://molmod.github.io/psiflow/
MIT License
121 stars 7 forks source link

Parsl version pin: is it needed? #8

Closed Andrew-S-Rosen closed 1 year ago

Andrew-S-Rosen commented 1 year ago

Hello, psiflow team! Thank you so much for the beautiful documentation on this package. I very much look forward to using this code in the future.

As for my question, I noticed that parsl is pinned to 2023.08.28 in pyproject.toml. Is this version pin necessary, or is it possible to relax this constraint?

svandenhaute commented 1 year ago

It's not needed! However, Parsl requires that manager and worker execute precisely the same version of the code. Because it has a weekly release cycle, it has happened quite often that when I set up the submission side and Apptainer container image at separate times, they install different versions of Parsl and become incompatible. Pinning the Parsl version ensures that people don't really have to worry about that anymore.

Andrew-S-Rosen commented 1 year ago

Ah, I see! That makes a lot of sense from a convenience standpoint. Good to know!

Andrew-S-Rosen commented 1 year ago

One limitation of the parsl version pin is that anyone who wants to use psiflow as a dependency can only use a specific version of Parsl, which might be limiting. Personally, I would recommend having specific version pins in a requirements.txt file that users would install from, which would allow for more flexibility than having them fixed in pyproject.toml, which should only be used for strict incompatibilities.

I've re-opened this issue since I think this might be worth further discussion.

svandenhaute commented 1 year ago

Tbh, it's fair to remove the version pin. Parsl throws a clear error when there's a version mismatch, and it can easily be fixed.

Andrew-S-Rosen commented 1 year ago

Great, thanks!