search

forsyde / forsyde-shallow

ForSyDe's Haskell-embedded Domain Specific Language
https://forsyde.github.io/forsyde-shallow/
BSD 3-Clause "New" or "Revised" License
12 stars 10 forks source link

Sadf library #10

Closed ricardobonna closed 6 years ago

ricardobonna commented 6 years ago

Hej Ingo and George,

I have finished the SADF Library and I'm pretty sure it's working beautifully. Here is the pull request. Let me know if you need something changed before accepting.

ingo-sander commented 6 years ago

@ugeorge: I suggest you let Ricardo demonstrate the SADF library and the CSDF library for you and then you can hopefully accept the pull request. The next step is then to add routines for automatic testing.

ugeorge commented 6 years ago

Sure, I'll check it out tomorrow and think of a minimal CI strategy. @ricardobonna, thanks for the great work!

ugeorge commented 6 years ago

@ricardobonna : could you please explain why the return type of the kernels (and other functions functions) of multiple outputs is list of tuple of list? The first list seems redundant, am I wrong?

For example, please explain the type signature of kernel12SADF? Or for that matter the signature of the output decoder function passed to detector11SADF, why does it have to yield [y] instead of y?

Based on your own examples, that list seems to be redundant, as it is always a singleton.

ricardobonna commented 6 years ago

That is a fair point, but the answer is that it is a necessary redundancy. The thing is that we are using maps and zipwiths in process constructors that yield more than one output (maps and zipwiths yield only one output). On Monday I can explain better, but for now, I would suggest you take a look at the SDF library actors with more than one output (say, actor12SDF).

And the thing about the detector11SADF yielding [y] instead of y is because the token rate of the kernel's output may be bigger than one. Say in some scenario, my kernel's output token rate is 2, then I need a function that returns [y1, y2]. That's why the list! Again, take a look at the SDF library because everything I did, I tried to mimic the SDF library because It would be better for the ForSyDe community to have everything coded with the same style.

We can discuss it better on Monday.

ugeorge commented 6 years ago

Hur hur, this does not mean that SDF is perfect :smile: . As @ingo-sander said, you must question everything. Take a look at forsyde-atom's SDF.comb22. That redundancy is certainly not necessary. That reminds me, I should check it out and if confirmed, I need to open an issue regarding the current SDF.

As for the detector yield... do I understand correct that you can yield multiple tokens? Then where is the production rate specified? detector11SDF specifies only a consumption rate, so I assume that the production rate is always 1. If this is not the case please take care of this.

ricardobonna commented 6 years ago

I guess you are right. It would be better if we got rid of these outer square brackets. I'm already taking care of it. Just be aware that we may need to change the SDF library and all the examples in the examples repository. And maybe we should check the untimed MoC as well as the dataflow MoC, as they may have the same issue.

As for the detector yield... I can explain it better on Monday. And if you still think it is going to be an issue, I will come up with a solution. Don't worry!

ingo-sander commented 6 years ago

There are a few aspects here.

  1. You should obviously question my implementation of the SDFlibrary.
  2. BUT there might also be reasons, why I did it that way. We need to understand this better. I also think that the modelling in the SDF library is quite convenient and straightforward, but if it can be improved then we should obviously rethink it here.
  3. As @ricardobonna pointed out, there are already quite some SDF examples, the most important the one used in the HW/SW Codesign handbook, where I think the actor22SDF is used. Please check also this example.

Continue the good discussion, which will lead to an even better understanding!

ugeorge commented 6 years ago

I remember thinking about this when I first implemented Atom, and @ingo-sander , if you remember, I was discussing in particular the role of "unzips" in ForSyDe process networks. I will try to look a bit into it and to refresh my memory, but I cannot promise anything before Monday.

ugeorge commented 6 years ago

@ricardobonna please check the issue with the production rate of the detectors. You may use wiki page of your own fork or the Slack channel to explain your solution. Support your solution with reference(s) to the SADF paper.