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

Remove non-native backends #1368

Merged
merged 6 commits into from
Aug 5, 2024
Merged

Remove non-native backends #1368

merged 6 commits into from
Aug 5, 2024

Conversation

alecandido
Copy link
Member

This just escaped a previous review.

The idea of the MetaBackend was to avoid any need for an explicit registration in qibo, so let's make it complete removing all mentions of "qibojit" & friends.

@alecandido
Copy link
Member Author

@BrunoLiegiBastonLiegi is there any reason to avoid listing "clifford" as an actual native backend?

@alecandido alecandido marked this pull request as ready for review June 20, 2024 15:58
Copy link

codecov bot commented Jun 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.99%. Comparing base (715339f) to head (a314126).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1368   +/-   ##
=======================================
  Coverage   99.99%   99.99%           
=======================================
  Files          78       78           
  Lines       11196    11199    +3     
=======================================
+ Hits        11195    11198    +3     
  Misses          1        1           
Flag Coverage Δ
unittests 99.99% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alecandido alecandido mentioned this pull request Jun 24, 2024
4 tasks
@BrunoLiegiBastonLiegi
Copy link
Contributor

Honestly, I don't remember right now why I decided to keep it separated, presumably because it is not properly an universal backend like the others and I wished to keep it separated, but I am not sure.

Regarding the updated list_available_backends function instead, I am not sure that I like the fact that you need to pass the name of the backends you wish to test. To me it would be better to have it search automatically for what is available, maybe, if we want to completely remove any information about which backends to look for, we could try generating a list of candidates through something like:

pip list | grep "^qibo.+"

@alecandido
Copy link
Member Author

Regarding the updated list_available_backends function instead, I am not sure that I like the fact that you need to pass the name of the backends you wish to test. To me it would be better to have it search automatically for what is available, maybe, if we want to completely remove any information about which backends to look for, we could try generating a list of candidates through something like:

pip list | grep "^qibo.+"

That's an option, of course.

The rationale behind the current proposal is that you could have any random package to expose backends for Qibo, irrespective of the name.
Of course, we could ask the package to start with qibo*, and just check those packages. We could test every package in pip list for the presence of a MetaBackend object (it will be definitely slower, but more because of specific packages, executing things on load - like qibojit - rather than because this list will be incredibly long).

So, the qibo* is definitely a valid alternative. Possibly even just for the default, but as the only mechanism.

@scarrazza scarrazza added this to the Qibo 0.2.9 milestone Jun 28, 2024
@alecandido
Copy link
Member Author

@BrunoLiegiBastonLiegi I was about implementing the thing we discussed with pip list, but I realized that in this way we would make the list dependent on the availability of the pip executable.
So, pip would become an implicit Qibo dependency.

Despite pip being generally available everywhere, I don't like so much having hidden dependencies at all.
We could parse ourselves the Python search locations, and identify packages. But even finding a snippet of code somewhere reliably doing that, it's a non-minor modification, so I would postpone to a later PR.

Would you mind merging as it is, for the sake of limiting dependency issues?
In any case, all of this is not going to survive qibo-core's new backends machinery, it was just a first step. And that's another reason why I'd prefer to work in a single direction (list_available_backends() was a nice interface feature I proposed, but it's not used anywhere ttbomu)

@alecandido alecandido force-pushed the remove-external-backends branch 2 times, most recently from 0a8290e to b0fabff Compare July 10, 2024 20:46
@MatteoRobbiati
Copy link
Contributor

Although I like the list_available_backends feature, I would proceed with this PR if in the direction of the qibo-core mechanism, since it will be useful anyway.
We can improve the documentation, for example by adding the backends diagram also in the frontpage, to guide the users through the backend choice.

@alecandido
Copy link
Member Author

alecandido commented Jul 12, 2024

Just for fun, I tried to implement the option for the qibo-core providers (that will be executables on the path) in a portable way.

import os
from pathlib import Path
from itertools import chain


def execs(start: str) -> list[str]:
    return [
        p.name
        for p in chain.from_iterable(
            Path(d).glob(f"{start}*") for d in os.environ["PATH"].split(os.pathsep)
        )
        if os.access(p, os.X_OK)
    ]


# test with few common starting strings
from pprint import pprint

pprint(execs("mount"))
pprint(execs("clang"))
pprint(execs("python"))
pprint(execs("system"))

This has no dependency other than the standard library :)
(and it relies on the "PATH" environment variable)

Of course, it will be invoked with execs("qibo-").

@scarrazza scarrazza modified the milestones: Qibo 0.2.9, Qibo 0.2.11 Jul 24, 2024
@alecandido alecandido added this pull request to the merge queue Aug 5, 2024
Merged via the queue into master with commit 540d2b6 Aug 5, 2024
27 checks passed
@alecandido alecandido deleted the remove-external-backends branch August 5, 2024 06:12
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.

None yet

5 participants