David-Durst / aetherling

Create auto-scheduled data-parallel pipelines in hardware with user-friendly Python
MIT License
12 stars 1 forks source link

Port Direction Issue #3

Open David-Durst opened 6 years ago

David-Durst commented 6 years ago

https://github.com/David-Durst/aetherling/blob/d6ce9d86b7a9f37eb846a47ff0bea63cb8ef8029/tests/test_map.py#L41 @leonardt I should be passing a definition here instead of an instance. However, I need to pass an instance so that the ports go in the right direction. On a definition, the in's have direction output and the outs have direction input. I know this is because this directionaly is for inside the definition, but what should I do here? Put in a hack where I just reverse directionality?

leonardt commented 6 years ago

What are you using the port direction for and how are using it? Could MapPartiallyParallel take in the definition and create an instance (or multiple instances) internally to use?

leonardt commented 6 years ago

Sorry it looks like you're referring to the argument to MapParallel, let me look at that code

leonardt commented 6 years ago

If i understand correctly,MapParallel takes in a Circuit and uses GetCoreIRModule to either get the wrappedModule or compile it to a coreir circuit. It looks like if it's an instance GetCoreIRModule gets the circuit's class, so either way the coreir backend compiles a definition. It's not clear to me where exactly you need the port directions to be a certain orientation? Shouldn't passing a definition through to the coreir backend work? Or am I missing a part of the pipeline that uses the fact that it's an instance.

phanrahan commented 6 years ago

I am trying to remember why I made the higher-order operators act on circuit instances not circuits.

I am pretty sure it had to do with passing in arguments to the constructors. Note that column get's passed an argument y. Sometimes constructors for each element depend on y. A good example is a LFSR. More generally, I have sometimes used generators to create instances and the generators take arguments.

David-Durst commented 6 years ago

@leonardt https://github.com/David-Durst/aetherling/blob/master/aetherling/modules/reduce.py#L72 is an example of how I need to operate on ports going in a specific direction. This function is renaming the input and output ports to match the names hardcoded in reduce. I do something similiar In MapParallel, but it's not an issue because I'm doing all the port logic in CoreIR.

David-Durst commented 6 years ago

@phanrahan For my purposes, there seems to be a lot more cons than pros for passing in instances. I need to make many instances usually (like Map and Reduce), so passing a single instance in is a mistake as I need to do a bunch of administrative work to remove it from the current definition and then get the instance's definition. How do your higher order operations get around this? Should you just pass in an array of args to higher-order operations for the instance constructors?

phanrahan commented 6 years ago

I agree, you don't want to pass in a single instance.

There are helper functions to make arrays of instances. For example,

def FFs(n, init=0, has_ce=False, has_reset=False):
    if isinstance(init, IntegerTypes):
        init = int2seq(init, n)
    def f(y):
        return FF(init[y], has_ce=has_ce, has_reset=has_reset)
    return col(f, n)

col creates n instances and returns it as a list.

Another reason for this interface is that the instances can have structure. For example, Ross mentioned that he wanted a 2D braid function. I would do this by creating a 2D list of list of instances.

leonardt commented 6 years ago

@David-Durst you could use the flip() method on the definition types to get the inverted type if you need access to the "alternate view". Basically if you have a definition but you want the instance view of the type you could do something like

    for name, value in i0.ports.items():
        print(name, type(value).flip())

We could add a method to Interface to get the flipped version, would that help?

leonardt commented 6 years ago

Turns out there's a method that already does this for you inteface.decl(), see https://github.com/phanrahan/magma/blob/91a73a80086f0f5ad5f3036b7f1226f7b3c8c525/magma/interface.py#L119-L123

It returns a list of the form name, type(port).flip(), ..., which matches the current interface for declaring Interfaces. We could change this to an ordereddict, but that will probably be bulked with the changes to the entire Interface logic to support OrderedDict