cog-imperial / OMLT

Represent trained machine learning models as Pyomo optimization formulations
Other
257 stars 56 forks source link

OMLT 1.0 missing required dependency #96

Closed jsiirola closed 1 year ago

jsiirola commented 1 year ago

I believe that there is a missing required dependency in the OMLT 1.0 release: onnx is listed as a "testing" dependency, but is required in order to import anything from omlt.io.

Steps to reproduce:

% pip install --user omlt
Collecting omlt
  Downloading omlt-1.0-py2.py3-none-any.whl (29 kB)
Requirement already satisfied: importlib-metadata in [...] (from omlt) (4.8.1)
Requirement already satisfied: numpy in [...] (from omlt) (1.21.2)
Requirement already satisfied: pyomo in [...] (from omlt) (6.4.3.dev0)
Requirement already satisfied: networkx in [...] (from omlt) (2.6.3)
Requirement already satisfied: typing-extensions>=3.6.4 in [...] (from importlib-metadata->omlt) (3.10.0.2)
Requirement already satisfied: zipp>=0.5 in [...] (from importlib-metadata->omlt) (3.4.0)
Requirement already satisfied: ply in [...] (from pyomo->omlt) (3.11)
Installing collected packages: omlt
Successfully installed omlt-1.0
% python
Python 3.7.12 (default, Nov 10 2021, 15:38:43)
[GCC 4.8.5 20150623 (Red Hat 4.8.5-44)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from omlt.io.keras_reader import load_keras_sequential
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "[...]/site-packages/omlt/io/__init__.py", line 1, in <module>
    from omlt.io.onnx import load_onnx_neural_network, write_onnx_model_with_bounds, load_onnx_neural_network_with_bounds
  File "[...]/site-packages/omlt/io/onnx.py", line 4, in <module>
    import onnx
ModuleNotFoundError: No module named 'onnx'

Possible solutions:

jalving commented 1 year ago

I see, even if you have tensorflow installed, it will hit the onnx code when you try to use the keras reader.

I think I'm in favor of either/both of the first two suggestions. Maybe we shouldn't be importing anything in omlt/io/__init__.py and also attempt what Pyomo does.

I remember talking with @carldlaird about this. We'll come up with something to clean this up.

jalving commented 1 year ago

@jsiirola this should now be fixed in main with PR #97. We are prepping to cut a new OMLT release soon.

tsaycal commented 1 year ago

Thanks @jalving. The PR has been incorporated into OMLT 1.1.