6
\$\begingroup\$

I am currently trying to better my understanding of python and good coding practices and would really like some input on a question i have been thinking over for weeks now.

I am currently working on building a virtual microprocessor out of basic logic gates (And, Or,...). The idea is to assemble more and more complex components out of parts that already exist. Once a component is created, i can reuse it whenever needed.

Almost all components are structured in the same way. They take one or more inputs and use these to create some inner logic which results in one or more outputs.

This component for example takes an 8-bit input (in_data) and inverts all its bits if another input (in_invert) carries a current:

class OnesComplement(IntegratedComponent):
 def __init__(self,
 in_data: list[ElectricComponent] = None,
 in_invert: ElectricComponent = None):
 self.in_data = in_data if in_data is not None else [
 LooseWire() for x in range(8)]
 self.in_invert = in_invert if in_invert is not None else LooseWire()
 self.out_main = [XOR(inp, self.in_invert) for inp in self.in_data]

Some inputs might not be available at initialization. In that a case one or more instances of LooseWire is created in order to keep track of all the components that depend on the missing input. This is also one of the reasons why the inputs are registered as instance attributes.

This code works perfectly fine, but it is very repetitive. In the example above, only a single line of code is used to implement the logic of the component. Everything else is just for setting defaults and registering instance attributes. This is annoying because I am writing a lot of components. It also leads to copy/paste errors and fails to highlight the occasional processing of non-input parameters.

As a way around this I tried using a decorator:

from functools import wraps
def _lwd(n=1):
 """
 ONLY for use with the @autoparse decorator.
 LooseWires are temporary and will be exchanged by the decorator.
 """
 if n == 1:
 return LooseWire()
 else:
 return [LooseWire() for _ in range(n)]
def autoparse(init):
 parnames = init.__code__.co_varnames[1:]
 defaults = init.__defaults__
 @wraps(init)
 def wrapped_init(self, *args, **kwargs):
 # Turn args into kwargs
 kwargs.update(zip(parnames[:len(args)], args))
 # apply default parameter values
 default_start = len(parnames) - len(defaults)
 for i in range(len(defaults)):
 if parnames[default_start + i] not in kwargs:
 kwargs[parnames[default_start + i]] = defaults[i]
 # generate new instance for each LooseWire
 for arg in kwargs:
 if isinstance(kwargs[arg], LooseWire):
 kwargs[arg] = LooseWire()
 if isinstance(kwargs[arg], list):
 for i in range(len(kwargs[arg])):
 if isinstance(kwargs[arg][i], LooseWire):
 kwargs[arg][i] = LooseWire()
 # attach attributes to instance
 for arg in kwargs:
 setattr(self, arg, kwargs[arg])
 init(self, **kwargs)
 return wrapped_init

This allows me to write the component from above like this:

class OnesComplement(IntegratedComponent):
 @autoparse
 def __init__(self,
 in_in: list[ElectricComponent] = _lwd(8),
 in_invert: ElectricComponent = _lwd()):
 self.out_main = [XOR(inp, self.in_invert) for inp in self.in_in]

I feel like this solves all of my problems. It is concise and also highlights the width of the input. However, i am worried about introducing some unwanted side-effects that i am not thinking about. Is it safe to decorate the __init__ like that? And if so, would it be considered good practice?

edit: more code as requested

code for the base class ElectricComponent:

class ElectricComponent:
 """
 Parent class for all components of the electric circuit
 """
 def connect_input(self, input_name: str,
 input_circuit: 'ElectricComponent'):
 """
 Should be used if not all inputs were available at initialization.
 Replaces LooseWires.
 """
 old_input = getattr(self, input_name)
 islist = isinstance(old_input, list)
 if not islist:
 old_input = [old_input]
 input_circuit = [input_circuit]
 for i in range(len(input_circuit)):
 for fc in old_input[i].forward_connections:
 setattr(fc[0], fc[1], input_circuit[i])
 input_circuit[i].add_connection(fc[0], fc[1])
 fc[0].update()
 if not islist:
 input_circuit = input_circuit[0]
 # Not functional, just for tracking
 setattr(self, input_name, input_circuit)

There are two types of components that inherit from ElectricComponent: CoreComponent and IntegratedComponent.

Most of the logic in maintaining connections and propagating state changes is handled by CoreComponents. They have their own state, stored as a bool in self.out_main.

class CoreComponent(ElectricComponent):
 """
 Parent Class for all core components
 These are the basic buidling blocks that all
 other components are assembled from
 All core components have a singlaur output-line
 called 'out_main'
 """
 def setup(self):
 self.forward_connections = []
 self.build_circuit()
 self.compute_state()
 def get_state(self):
 """Returns the current state of the output"""
 return self.out_main
 is_on = property(get_state)
 def add_connection(self, con, port):
 """Called by downstream elements to add them as a forward connection"""
 if (con, port) not in self.forward_connections:
 self.forward_connections.append((con, port))
 def forward_pass(self):
 for fc in self.forward_connections:
 fc[0].update()
 def update(self):
 old_state = self.out_main
 self.compute_state()
 if self.out_main != old_state:
 self.forward_pass()
 def __str__(self):
 return str(int(self.out_main))

All CoreComponents (for now):

class Switch(CoreComponent):
 """
 Simple Switch. It is used to control a gate.
 When the gate is closed it carries a current.
 """
 def __init__(self, closed: bool = False):
 self.out_main = closed
 self.setup()
 def build_circuit(self):
 pass
 def compute_state(self):
 pass
 def flip(self):
 self.out_main = not self.out_main
 self.forward_pass()
 def open(self):
 self.out_main = False
 self.forward_pass()
 def close(self):
 self.out_main = True
 self.forward_pass()
class LooseWire(CoreComponent):
 """
 This component is used solely for initializing unconnected inputs
 It never carries a current
 """
 def __init__(self):
 self.setup()
 def build_circuit(self):
 pass
 def compute_state(self):
 self.out_main = False
class BaseGate(CoreComponent):
 """
 Parent for the baisc logic gates
 """
 def __init__(self, in_a: ElectricComponent = None,
 in_b: ElectricComponent = None):
 self.in_a = in_a if in_a is not None else LooseWire()
 self.in_b = in_b if in_b is not None else LooseWire()
 self.setup()
 def build_circuit(self):
 self.in_a.add_connection(self, 'in_a')
 self.in_b.add_connection(self, 'in_b')
class INV(CoreComponent):
 """Inverts the input"""
 def __init__(self, in_a: ElectricComponent = None):
 self.in_a = in_a if in_a is not None else LooseWire()
 self.setup()
 def build_circuit(self):
 self.in_a.add_connection(self, 'in_a')
 def compute_state(self):
 self.out_main = True
 if self.in_a.is_on:
 self.out_main = False
class AND(BaseGate):
 """AND-Gate"""
 def compute_state(self):
 self.out_main = False
 if self.in_a.is_on:
 if self.in_b.is_on:
 self.out_main = True
class OR(BaseGate):
 """OR-Gate"""
 def compute_state(self):
 self.out_main = False
 if self.in_a.is_on:
 self.out_main = True
 if self.in_b.is_on:
 self.out_main = True
class NOR(BaseGate):
 """NOR-Gate"""
 def compute_state(self):
 self.out_main = True
 if self.in_a.is_on:
 self.out_main = False
 if self.in_b.is_on:
 self.out_main = False
class NAND(BaseGate):
 """NAND-Gate"""
 def compute_state(self):
 self.out_main = True
 if self.in_a.is_on:
 if self.in_b.is_on:
 self.out_main = False

The other class that inherits from ElectricComponent is IntegratedComponent. Components of this type are assembled out of CoreComponents. Most of the components i build are of this type. They do not posses a state of their own. The class itself is empty for now. It used to contain some methods i factored out but i am keeping it around for structuring and future use.

There are too many IntegratedComponents to post them all. One example of an IntegratedComponent would be the OnesComplement i originally posted. Another is the XOR-gate which is used in the OnesComplement

class IntegratedComponent(ElectricComponent):
 pass
class IntegratedLogicGate(IntegratedComponent):
 # TODO: think about moving this to a class for all 
 # IntegratedComponents with a single 'out_main'
 """
 This allows for its children to be used
 like basegates and other core components
 and1 = AND(s1)
 xor1 = XOR(s1, s2)
 xor.out_main.is_on can be written as
 xor.is_on
 and1.connect_input('in_b', xor1.out_main)
 can be written as if XOR were a CoreComponent
 and1.connect_input('in_b', xor1)
 """
 def get_state(self):
 return self.out_main.get_state()
 is_on = property(get_state)
 def add_connection(self, con, port):
 self.out_main.add_connection(con, port)
class XOR(IntegratedLogicGate):
 """XOR-Gate"""
 def __init__(self, in_a: ElectricComponent = None,
 in_b: ElectricComponent = None):
 self.in_a = in_a if in_a is not None else LooseWire()
 self.in_b = in_b if in_b is not None else LooseWire()
 self.or1 = OR(self.in_a, self.in_b)
 self.nand1 = NAND(self.in_a, self.in_b)
 self.out_main = AND(self.or1, self.nand1)

One example of a component that needs a LooseWire because it has its own output as an input is the RSFlipFlop

class RSFlipFlop(IntegratedComponent):
 def __init__(self,
 in_r: ElectricComponent = None,
 in_s: ElectricComponent = None):
 self.in_r = in_r if in_r is not None else LooseWire()
 self.in_s = in_s if in_s is not None else LooseWire()
 self.nor1 = NOR(self.in_r)
 self.nor2 = NOR(self.nor1, self.in_s)
 self.nor1.connect_input("in_b", self.nor2)
 self.out_q = self.nor1
 self.out_qb = self.nor2

edit 2:

I should have been more clear as to what my goals for this project are. I am currently reading trough Charles Petzold's book Code: The Hidden Language of Hardware and Software. In it, he is shows how to assemble a microprocessor out of basic logic gates constructed from relays and switches. I am trying to replicate this virtually. The 5 logic gates I use as core components are the 5 gates he uses as building blocks in his book. This is also why those gates only have two inputs and why i chose nested if-clauses for them. I can't use logic operators before building them myself.

asked Oct 31, 2021 at 11:40
\$\endgroup\$
1
  • \$\begingroup\$ @Reinderien I added more code as requested \$\endgroup\$ Commented Nov 1, 2021 at 10:36

1 Answer 1

4
\$\begingroup\$

In no particular order:

  • Your input model is too complicated. Don't isinstance(old_input, list); just assume that every component has a Collection[] (sized, iterable, unordered) of (potentially only one) input.
  • Similarly, your output model is sometimes too complicated. There's not a lot of benefit in representing your OnesComplement as a byte bus - just represent it as a single bit (which really just evaluates to a XOR). If you want to generalize an arbitrary gate to a byte bus you should do that separately, possible for all components, not just OnesComplement.
  • CoreComponent is effectively an abstract class due to compute_state not being defined - fine. However, build_circuit(self): pass should exist on CoreComponent itself, and compute_state should also be declared there as compute_state(self): raise NotImplementedError().
  • Relying on getattr/setattr as much as this code does is a code smell.
  • In most cases, it doesn't benefit you to hard-code in_a / in_b. It's less work for an OR, AND etc. to just accept an arbitrary number of inputs.
  • Don't auto-convert in_a from a None to a LooseWire; just accept a component that can never be None. Also, your in_a annotation is currently incorrect because it's missing an Optional[]. Further: you probably shouldn't allow for a LooseWire at all.
  • build_circuit and add_connection probably shouldn't exist. You should just be able to assume that your inputs are your connections.
  • As to your original question, no, your decorator is really not a good idea. Magic conversion of constructor parameters is more complicated and difficult to understand than it should be.

State and forward propagation

All but two of your components - RSFlipFlop and Switch - should be stateless - just as they are in real life. As such, it is not a good idea to hold a self.out_main. Just define an evaluation function, basically your compute_state, that returns a boolean.

It's not a good idea to keep forward_connections, at least in its current form. In real life, the behaviour of a component should not rely on the components after it.

Since you're encountering performance issues, it is possible to be a little more clever (I have not shown that in the suggested code):

  • Approach this in a general manner, keeping only one boolean output: this includes your FF, where you can simplify its model to only have non-inverting output. If you need an inverter afterward, keep that as a separate component.
  • Cache the old value of the output, similar to your out_main.
  • Only forward-propagate changes if the old value is not equal to the new value. You had already done this (albeit a little inconsistently) in if self.out_main != old_state.
  • Use __slots__.
  • Consider finalizing a circuit using run-time code objects and compile.

Suggested code

from functools import reduce
from operator import xor
from sys import maxsize
from typing import ClassVar, Collection, Callable, Iterable, List
class Component:
 """
 Parent class for all components of the electric circuit
 """
 def __init__(self) -> None:
 self.change_hooks: List[Callable[[], None]] = []
 @property
 def out(self) -> bool:
 raise NotImplementedError()
 def __bool__(self) -> bool:
 return self.out
 def changed(self) -> None:
 for hook in self.change_hooks:
 hook()
class InputComponent(Component):
 def __init__(self, inputs: Iterable['Component']) -> None:
 super().__init__()
 for input in inputs:
 input.change_hooks.append(self.changed)
class InputCollectionComponent(InputComponent):
 MIN_INPUTS: ClassVar[int]
 MAX_INPUTS: ClassVar[int]
 def __init__(self, inputs: Collection['Component']) -> None:
 super().__init__(inputs)
 if not (self.MIN_INPUTS <= len(inputs) <= self.MAX_INPUTS):
 raise ValueError(f'{len(inputs)} inputs for {type(self).__name__} '
 f'not between {self.MIN_INPUTS}-{self.MAX_INPUTS}')
 self.inputs = inputs
class Switch(Component):
 def __init__(self, closed: bool) -> None:
 super().__init__()
 self.closed = closed
 def flip(self) -> None:
 self.closed = not self.closed
 self.changed()
 @property
 def out(self) -> bool:
 return self.closed
class INV(InputComponent):
 def __init__(self, input: Component) -> None:
 super().__init__((input,))
 self.input = input
 @property
 def out(self) -> bool:
 return not self.input
class Gate(InputCollectionComponent):
 MIN_INPUTS = 2
 MAX_INPUTS = maxsize
class AND(Gate):
 @property
 def out(self) -> bool:
 return all(self.inputs)
class OR(Gate):
 @property
 def out(self) -> bool:
 return any(self.inputs)
class NOR(Gate):
 @property
 def out(self) -> bool:
 return not any(self.inputs)
class NAND(Gate):
 @property
 def out(self) -> bool:
 return not all(self.inputs)
class XOR(Gate):
 @property
 def out(self) -> bool:
 return reduce(xor, self.inputs, False)
class RSFlipFlop(InputComponent):
 def __init__(self, *, in_r: Component, in_s: Component) -> None:
 super().__init__((in_r, in_s))
 self.in_r, self.in_s = in_r, in_s
 self.update()
 @property
 def out(self) -> bool:
 return self.state
 def update(self) -> None:
 self.state = self.in_s.out and not self.in_r.out
 def changed(self) -> None:
 self.update()
 super().changed()
class OnesComplement(XOR):
 def __init__(self, in_in: Component, in_invert: Component) -> None:
 super().__init__((in_in, in_invert))
def test() -> None:
 swa = Switch(closed=False)
 swb = Switch(closed=False)
 or_ = OR((swa, swb))
 ff = RSFlipFlop(in_r=swa, in_s=or_)
 assert not swa
 assert not swb
 assert not or_
 assert not ff
 swb.flip()
 assert not swa
 assert swb
 assert or_
 assert ff
if __name__ == '__main__':
 test()
answered Nov 1, 2021 at 15:46
\$\endgroup\$
3
  • \$\begingroup\$ Thank you for taking the time to read trough this and respond. I very much look forward to your suggested implementation. Fwiw, I did start out without forward connections and everything being stateless, but as the circuits grew bigger this started to become slow because i was recursing trough the whole circuit for every bit in the output. This is why i introduced the forward connections. The idea was that a change in component A would only affect the components (forward_connections) that depend on the state of component A and that forward propagation would stop for unaffected components. \$\endgroup\$ Commented Nov 1, 2021 at 16:51
  • \$\begingroup\$ Thanks again. Many good pointers. I learned a lot from this and will try my hands at a recursive solution again. Unfortunately your code does not really work for what i'm trying to achieve (see my latest edit). I'm sorry i wasn't more clear about that. \$\endgroup\$ Commented Nov 2, 2021 at 8:23
  • \$\begingroup\$ @LoePhi That's.. fine, but if you want a review that takes the new information into account you're going to have to repost. \$\endgroup\$ Commented Nov 2, 2021 at 13:17

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.