qiboteam / qibotn

The tensor-network translation module for Qibo.
https://qibo.science
Apache License 2.0
3 stars 1 forks source link

Suggestion: Transition from setup.py to Poetry for Better Package and Dependency Management #23

Closed yjmaxpayne closed 4 months ago

yjmaxpayne commented 5 months ago

@liweintu: Qibo repositories have mostly shifted to Poetry. In our ongoing effort to improve the development process and maintainability of our project, we are considering transitioning from setup.py to Poetry for package and dependency management.

Poetry provides a more modern and intuitive approach to managing Python packages. It simplifies the process of setting up a new project, managing dependencies, and building and publishing packages.

Thus, I'd like to put up this issue as a tracker for the team to consider shifting to poetry when the majority of the development tasks are completed to support the release of QiboTN.

liweintu commented 5 months ago

Adopting poetry is not a problem, as is already discussed and implemented in qibo repo. It's just that, when we later integrate qibotn backend into qibo, we'll naturally follow qibo's poetry set-up. So I would say we put the adoption when integrating qibotn into qibo rather than spending twice the effort doing transitioning.

alecandido commented 5 months ago

It's just that, when we later integrate qibotn backend into qibo,

Actually, what do you mean exactly by integrating qibotn into qibo?

rather than spending twice the effort doing transitioning.

Just because I don't understand what should change later on.

In principle, for Qibojit and Qibolab they live as stand-alone packages, just exposing a Qibo backend (import qibo and define a class that inherits from the AbstractBackend, or more frequently from the NumpyBackend - to be improved in the near future). And then there will be an if in Qibo that internally is importing the package and instantiating the backend.

The only thing that could change is a dependency on qibo, but in practice it's already there because it is used for Quimb conversion: https://github.com/qiboteam/qibotn/blob/8e2db6813f5b74468ec778a56ec5cfb937a3b626/src/qibotn/quimb.py#L3

For sure I'm missing something...

liweintu commented 5 months ago

Actually, what do you mean exactly by integrating qibotn into qibo?

By "integrating qibotn into qibo", I mean the merging the refactored qibotn code into qibo repo.

In principle, for Qibojit and Qibolab they live as stand-alone packages, just exposing a Qibo backend (import qibo and define a class that inherits from the AbstractBackend, or more frequently from the NumpyBackend - to be improved in the near future). And then there will be an if in Qibo that internally is importing the package and instantiating the backend.

This is what we have done now, creating an if branch in Qibo to branch into qibotn backend for TN-enabled simulation. We can keep qibotn this way as stand-alone package. But for future, do we consider merging qibotn package into qibo package? Similarly, for Qibojit and Qibolab as well, do we consider merging them into Qibo package? If this is the plan, then when the merging starts, poetry will be naturally adopted.

The only thing that could change is a dependency on qibo, but in practice it's already there because it is used for Quimb conversion:

Yes, you're correct that qibotn only needs to depend on qibo by design, nothing else.

alecandido commented 5 months ago

By "integrating qibotn into qibo", I mean the merging the refactored qibotn code into qibo repo.

Ok, this is what I suspect, but I didn't expect to happen. To my understanding, qibotn should live as a separate repo a la qibojit.

But for future, do we consider merging qibotn package into qibo package? Similarly, for Qibojit and Qibolab as well, do we consider merging them into Qibo package? If this is the plan, then when the merging starts, poetry will be naturally adopted.

I believe there is no plan to have a monorepo. Things are being managed as separate repositories.

Yes, you're correct that qibotn only needs to depend on qibo by design, nothing else.

This could be relevant (at some point) https://github.com/qiboteam/qibo/issues/1117

liweintu commented 5 months ago

Ok, this is what I suspect, but I didn't expect to happen. To my understanding, qibotn should live as a separate repo a la qibojit.

Yes, this is the key point. If the plan is to keep it separate as is, then we just raise a PR to qibo to add the if branch for qibotn first.

This could be relevant (at some point) https://github.com/qiboteam/qibo/issues/1117

Thanks for the pointer. This is relevant because it's also about dependencies. Like the current case, we need two PRs (one for qibotn, the other for qibo) to make it compatible to invoke qibotn from qibo package. Thus, the versions of qibotn and qibo need to be compatible. We encountered cases where our developed qibotn code worked for some versions of qibo, but not some other versions (mostly higher versions) of qibo. So, if this pointer can address the versioning of the two packages, that'd be nice.

alecandido commented 5 months ago

Thanks for the pointer. This is relevant because it's also about dependencies. Like the current case, we need two PRs (one for qibotn, the other for qibo) to make it compatible to invoke qibotn from qibo package. Thus, the versions of qibotn and qibo need to be compatible. We encountered cases where our developed qibotn code worked for some versions of qibo, but not some other versions (mostly higher versions) of qibo. So, if this pointer can address the versioning of the two packages, that'd be nice.

I believe the answer is "yes and no".

It will solve part of the dependency issue, because you will be able to depend on qibo-core, where the backend mechanism will be defined, and qibo will be able to depend on qibotn. Thus, you can fix the supported version of qibotn inside the qibo front-end, and the dependency resolution will enforce the compatibility for you (with the manual, natural, input of the supported qibo-core version range in qibotn).

However, if you will develop a new backend you will always need the second PR for the registration of the backend in qibo. In principle, you can always depend on a new package just in user code, like qibo-new-backend, and instantiate the NewBackend (or even just define the class in user code) and then dynamically set this backend, passing the object instead of a string. This second part would be better addressed by a plugin mechanism, in which new Qibo backends will be able to register themselves on import* (extending those of which Qibo is already aware).

* it is possible even without import, just relying on conventional names and Qibo scanning the available packages. But I'm not sure this is advisable, though it can support situations in which you are not directly using Qibo through Python, like dispatching via CLI.

liweintu commented 4 months ago

Closing now as it's completed in PR #26 .