-
Notifications
You must be signed in to change notification settings - Fork 419
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
Lazy converter imports #1094
base: main
Are you sure you want to change the base?
Lazy converter imports #1094
Conversation
06bf84c
to
bf7485f
Compare
I agree with the spirit of the change but not the execution. Optional dependencies are the way to go but not unchecked lazy imports. Having imports randomly fail in the middle of some function and leaving the user to deal with the problem themselves is not friendly. In the original proposal for modular hls4ml, frontends and backends would be installable extras. All of them. In practice this would mean basic install of hls4ml would only provide the core classes that can be used to save/load hls4ml models (a feature currently in development) and one backend (which has to be enforced somehow in To achieve that we should make sure that our internals don't depend on big external libraries, in addition to some reordering of the imports that this PR does. To that end, we should focus on removing qkeras as the internal dependency from types and from profiling functions. Since we're already discussing ditching QKeras official for a drop-in replacement library, we can replace the internal dependency with chang's reusable optimizers, which mimic qkeras anyway. Then this new replacement library (yet to be named) can be built on top of the optimizers library. It would contain the definitions of layers (QDense, QConv1D/2D etc) that rely on our optimizers. Our keras frontend could then depend on this library, if even needed (as it seems that we only really need qkeras' optimizers for parsing, not the layer definitions). |
I was thinking a missing import is rather self-explanatory. Here are few proposals to throw an informative error for this without large restructuring for the short to midterm. Restructuring the whole project to multiple frontend and backend sounds like an overkill for this particular issue at the moment.
For types & quantizers, the The standalone
|
Description
Make all front-end specific dependencies (
tf
,(q)keras
,torch
,onnx
) optional and imported lazily.All handlers are registered now, no matter if the frontend specific package is present. The frontend specific packages are only imported if the corresponding handler is used. This should remove the strong dependency of hls4ml on
qkeras
andtensorflow
(e.g., when converting a model not intf.keras
, tensorflow no longer need to be installed).Type of change
Tests
Test Configuration:
pre-commit
on the files I edited or added.