leonardt / fault

A Python package for testing hardware (part of the magma ecosystem)
BSD 3-Clause "New" or "Revised" License
41 stars 13 forks source link

Add analog type to magma for fault mixed signal types #232

Open leonardt opened 4 years ago

leonardt commented 4 years ago

Related discussion https://github.com/leonardt/fault/pull/231

rsetaluri commented 4 years ago

I think the type hierarchy should look like:

I think we should move away from the pattern of isisntance(x, Digital) and have generic functions, like is_digital() etc defined on all types. And we can use mixins to get more mileage out of this. As it stands there's no real distinction between values of Digital, Real, Elect, etc. in terms of the methods you can call on them and what they return. Basically, I'm suggesting that sub-typing should be used to extend/override specific functionality, rather than just to convey semantics.

sgherbst commented 4 years ago

Hierarchy looks good; only thing I would recommend is to put Real and Elect under a type called Analog. Real and Elect both represent analog signals, but use different simulator features -- Real is Verilog real type (i.e., event-driven) whereas Elect is a true SPICE net (i.e., variable in diff. eq. solver). If a user specifies only that a signal is Analog, that could resolve to Real or Elect depending on defaults for a certain kind of fault target, user preferences, overrides, etc.

I also thought it might be interesting to highlight a few ways that typing is used in fault with regard to analog signals. (Since I'm not too familiar with the type system, some of these may not be the "right" way...)

Example 1: A type is checked to see if it is a subclass of RealType in generate_port_code(..., name, type_, ...) https://github.com/leonardt/fault/blob/1abc49b8ec66bad372e5728d2a2f42ab1b1264fa/fault/system_verilog_target.py#L550-L551

Example 2: A circuit port is checked to see if it is an instance of RealType. (I gather that type_ in the previous example was a class whereas a port is an instance of a class?) https://github.com/leonardt/fault/blob/1abc49b8ec66bad372e5728d2a2f42ab1b1264fa/fault/spice_target.py#L392-L394

Example 3: A port is checked to see if it is an instance of Real/Elect In/Out/InOut. Based on the previous example, maybe isinstance(self.port, (RealType, ElectType)) would work & be more generic? https://github.com/leonardt/fault/blob/1abc49b8ec66bad372e5728d2a2f42ab1b1264fa/fault/actions.py#L103-L106

Example 4: A type_ is checked to see if it refers to ElectIn/ElectOut/etc. Based on example 1, maybe issubclass(type_, ElectType) would work and be more generic? https://github.com/leonardt/fault/blob/1abc49b8ec66bad372e5728d2a2f42ab1b1264fa/fault/verilogams.py#L69-L77