adding profile file for backends #457
Conversation
Codecov Report
@@ Coverage Diff @@
## master #457 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 85 85
Lines 11851 11867 +16
=========================================
+ Hits 11851 11867 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thank you for implementing this, I believe it is a great first step to simplify the process of adding new backends, especially from external collaborators. I think now it is possible to add a backend without requiring to touch the qibo internals at all, which is great, however one should still inherit Qibo's abstract backend and define the corresponding methods.
With that in mind, I believe that as next step we should better document and perhaps somehow simplify this process, for example by making clear which methods are required and optional (eg. the multi-GPU methods could be optional). We should also improve the hardware abstractions, as currently the abstract backend is geared during simulation.
src/qibo/tests/conftest.py
Outdated
| @@ -11,9 +11,9 @@ | |||
| _ACCELERATORS = None | |||
| if ("qibotf" in _available_backends or "qibojit" in _available_backends): | |||
| _ACCELERATORS = "2/GPU:0,1/GPU:0+1/GPU:1,2/GPU:0+1/GPU:1+1/GPU:2" | |||
| if "icarusq" in _available_backends: # pragma: no cover | |||
| if "qiboicarusq" in _available_backends: # pragma: no cover | |||
There was a problem hiding this comment.
We can make this backend name agnostic by using the .is_hardware property (which perhaps we need to add to AbstractBackend). Alternatively something that could work and skip the tests for all hardware backends would be to change the _available_backends definition to:
_available_backends = set(qibo.K.available_backends.keys()) - set(qibo.K.hardware_backends.keys())In a similar manner, we could possibly extend this PR to completely remove hard-coded backend names by adding the relevant properties in the abstract backend. For example another case which currently requires hard-coded backend names around the code is the multi-GPU (see the if "qibotf" in _available_backends: above). We could make this name agnostic using a .supports_multigpu property in the backends. The multi-GPU case may be slightly more complicated than .is_hardware so we should probably to this in a different PR, but let me know what you think.
Co-authored-by: Stavros Efthymiou <[email protected]>
|
Thanks for the review. Indeed, this PR should be the basis for a more general cleanup and simplification. |
Thank you for the updates, looks good. Just don't forget to change the base branch to |
As discussed, this PR implements the backend loading mechanism using a default configuration file located in
qibo/backends/profiles.ymlor by using theQIBO_PROFILEenvironment variable.At some point in the future, we should consider the possibility to store the backend class definition directly in the external module.