Skip to content
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

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

Open
leonardt opened this issue Apr 4, 2020 · 2 comments
Open

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

leonardt opened this issue Apr 4, 2020 · 2 comments
Assignees

Comments

@leonardt
Copy link
Owner

leonardt commented Apr 4, 2020

Related discussion #231

@rsetaluri rsetaluri self-assigned this Apr 4, 2020
@rsetaluri
Copy link
Collaborator

I think the type hierarchy should look like:

  • Signal
    • Digital (for backward compat)
      • Bit
      • Clock
      • ...
    • Real
    • Elect
    • ...

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
Copy link
Collaborator

sgherbst commented Apr 4, 2020

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_, ...)

if issubclass(type_, RealType):
t = "real"

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?)

fault/fault/spice_target.py

Lines 392 to 394 in 1abc49b

port = self.circuit.interface.ports[port_name]
if isinstance(port, (m.Bit, fault.RealType, fault.ElectType)):
retval += [f'{port}']

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?

fault/fault/actions.py

Lines 103 to 106 in 1abc49b

@property
def real_number_port(self):
return isinstance(self.port, (RealIn, RealOut, RealInOut,
ElectIn, ElectOut, ElectInOut))

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?

# determine port type
if type_ is ElectIn or type_ is ElectOut:
vams_type = 'electrical'
elif type_ is RealIn or type_ is RealOut:
vams_type = 'wreal'
elif len(type_) == 1:
vams_type = 'wire'
else:
vams_type = f'wire [{len(type_) - 1}:0]'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants