search

es-ude / elastic-ai.creator

elastic ai.creator
MIT License
16 stars 2 forks source link

`DesignCreator` should inherit from `torch.nn.Module` #369

Closed julianhoever closed 3 months ago

julianhoever commented 3 months ago

The abstract class DesignCreator should also inherit from torch.nn.Module because it is used to ensure that a torch.nn.Module always have a create_design function for translation. Therefore it is mostly used in combination with a torch.nn.Module.

https://github.com/es-ude/elastic-ai.creator/blob/662e45aaaa1c70bb1ffb9e30f69a389289000d44/elasticai/creator/vhdl/design_creator.py#L6-L8

In addition in the Sequential class we assume that submodules of type DesignCreator passed to the constructor are also torch.nn.Module, because we cast them to torch.nn.Module which is not possible if the submodules of type DesignCreator not inherit from torch.nn.Module.

https://github.com/es-ude/elastic-ai.creator/blob/662e45aaaa1c70bb1ffb9e30f69a389289000d44/elasticai/creator/nn/sequential/layer.py#L12-L27

Perhaps the name DesignCreator is not a good choice.

glencoe commented 3 months ago

I'm not sure i agree. For testing purposes and modularity, i like the two to be separated. But i agree that adding Module there would communicate intent better. What do you think about introducing a DesignCreatorModule or just CreatorModule (that users should inherit from) and keep the DesignCreator as is?