tango icon indicating copy to clipboard operation
tango copied to clipboard

Steps cannot take `Model` as input

Open AkshitaB opened this issue 3 years ago • 1 comments

Right now, steps cannot take Model object as inputs, unless _to_params() is implemented. As it's unreasonable to expect users to write _to_params() for every implementation, we can add any empty implementation of it in the base class. However, that may lead to incorrect hashing for the object potentially?

Note: the actual layers in the model don't matter.

Error observed:

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../tango/common/testing.py:87: in run
    run_name = _run(
../../tango/__main__.py:707: in _run
    run = workspace.register_run((step for step in step_graph.values()), name)
../../tango/workspaces/local_workspace.py:345: in register_run
    all_steps = set(targets)
../../tango/step.py:452: in __hash__
    return hash(self.unique_id)
../../tango/step.py:431: in unique_id
    self.unique_id_cache += det_hash(
../../tango/common/det_hash.py:128: in det_hash
    pickler.dump(o)
../../../../../virtuals/tango/lib/python3.8/site-packages/dill/_dill.py:498: in dump
    StockPickler.dump(self, obj)
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:485: in dump
    self.save(obj)
../../tango/common/det_hash.py:102: in save
    super().save(obj, save_persistent_id)
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:558: in save
    f(self, obj)  # Call unbound method with explicit self
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:884: in save_tuple
    save(element)
../../tango/common/det_hash.py:102: in save
    super().save(obj, save_persistent_id)
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:558: in save
    f(self, obj)  # Call unbound method with explicit self
../../../../../virtuals/tango/lib/python3.8/site-packages/dill/_dill.py:990: in save_module_dict
    StockPickler.save_dict(pickler, obj)
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:969: in save_dict
    self._batch_setitems(obj.items())
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:1000: in _batch_setitems
    save(v)
../../tango/common/det_hash.py:102: in save
    super().save(obj, save_persistent_id)
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:537: in save
    pid = self.persistent_id(obj)
../../tango/common/det_hash.py:107: in persistent_id
    det_hash_object = obj.det_hash_object()
../../tango/common/from_params.py:838: in det_hash_object
    r = (self.__class__.__qualname__, self.to_params())
../../tango/common/from_params.py:820: in to_params
    return Params(replace_object_with_params(self._to_params()))
../../tango/common/from_params.py:809: in replace_object_with_params
    return {key: replace_object_with_params(value) for key, value in o.items()}
../../tango/common/from_params.py:809: in <dictcomp>
    return {key: replace_object_with_params(value) for key, value in o.items()}
../../tango/common/from_params.py:809: in replace_object_with_params
    return {key: replace_object_with_params(value) for key, value in o.items()}
../../tango/common/from_params.py:809: in <dictcomp>
    return {key: replace_object_with_params(value) for key, value in o.items()}
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

o = Sequential(
  (0): FeedForward(
    (linear): Linear(in_features=4, out_features=4, bias=True)
    (activation): ReLU(...)
  )
  (2): FeedForward(
    (linear): Linear(in_features=4, out_features=4, bias=True)
    (activation): ReLU()
  )
)

    def replace_object_with_params(o: Any) -> Any:
        if isinstance(o, FromParams):
            return o.to_params().as_dict(quiet=True)
        elif isinstance(o, (list, tuple, set)):
            return [replace_object_with_params(i) for i in o]
        elif isinstance(o, dict):
            return {key: replace_object_with_params(value) for key, value in o.items()}
        elif isinstance(o, Path):
            return str(o)
        elif o is None or isinstance(o, (str, float, int, bool)):
            return o
        else:
>           raise NotImplementedError(
                f"Unexpected type encountered in to_params(): {o} ({type(o)})\n"
                "You may need to implement a custom '_to_params()'."
            )
E           NotImplementedError: Unexpected type encountered in to_params(): Sequential(
E             (0): FeedForward(
E               (linear): Linear(in_features=4, out_features=4, bias=True)
E               (activation): ReLU()
E             )
E             (1): FeedForward(
E               (linear): Linear(in_features=4, out_features=4, bias=True)
E               (activation): ReLU()
E             )
E             (2): FeedForward(
E               (linear): Linear(in_features=4, out_features=4, bias=True)
E               (activation): ReLU()
E             )
E           ) (<class 'torch.nn.modules.container.Sequential'>)
E           You may need to implement a custom '_to_params()'.

../../tango/common/from_params.py:815: NotImplementedError

AkshitaB avatar Apr 15 '22 07:04 AkshitaB

Note to self: Can we get rid of to_params completely?

dirkgr avatar Apr 28 '22 21:04 dirkgr