forsyde / IDeSyDe

Design space identification and exploration
https://forsyde.github.io/IDeSyDe
4 stars 4 forks source link

Specifying token size for an SDFChannel crashes the PL decision model #34

Closed BeethovenKodar closed 5 months ago

BeethovenKodar commented 6 months ago

Same platform as in examples_and_benchmarks/novel/programmable_logic_area/MPSoC_Final.fiodl and application is attached

evaluatorsdf.txt

Running idesyde with these input files produces

thread '<unnamed>' panicked at rust-bridge-minizinc/src/lib.rs:790:38:
Should not fail to parse the output of minizinc: Error("missing field `output`", line: 1, column: 2155)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Rayon: detected unexpected panic; aborting
./run.sh: line 60: 1070663 Aborted                 (core dumped) ./$DSE_EXECUTABLE -v DEBUG -p 5 --x-total-time-out 30 $plat $appl
BeethovenKodar commented 6 months ago
{
    "type": "warning",
    "stack": [
        {
            "location": {
                "filename": "/tmp/idesyde/minizinc/AADPMMMPL.mzn",
                "firstLine": 63,
                "firstColumn": 12,
                "lastLine": 65,
                "lastColumn": 1
            },
            "isCompIter": false,
            "description": "call 'forall'"
        },
        {
            "location": {
                "filename": "/tmp/idesyde/minizinc/AADPMMMPL.mzn",
                "firstLine": 63,
                "firstColumn": 12,
                "lastLine": 65,
                "lastColumn": 1
            },
            "isCompIter": false,
            "description": "array comprehension expression"
        },
        {
            "location": {
                "filename": "/tmp/idesyde/minizinc/AADPMMMPL.mzn",
                "firstLine": 63,
                "firstColumn": 19,
                "lastLine": 63,
                "lastColumn": 19
            },
            "isCompIter": true,
            "description": "b = <expression>"
        },
        {
            "location": {
                "filename": "/tmp/idesyde/minizinc/AADPMMMPL.mzn",
                "firstLine": 63,
                "firstColumn": 33,
                "lastLine": 63,
                "lastColumn": 33
            },
            "isCompIter": true,
            "description": "p = <expression>"
        },
        {
            "location": {
                "filename": "/tmp/idesyde/minizinc/AADPMMMPL.mzn",
                "firstLine": 63,
                "firstColumn": 49,
                "lastLine": 63,
                "lastColumn": 50
            },
            "isCompIter": true,
            "description": "pe = <expression>"
        },
        {
            "location": {
                "filename": "/tmp/idesyde/minizinc/AADPMMMPL.mzn",
                "firstLine": 63,
                "firstColumn": 65,
                "lastLine": 63,
                "lastColumn": 66
            },
            "isCompIter": true,
            "description": "me = <expression>"
        },
        {
            "location": {
                "filename": "/tmp/idesyde/minizinc/AADPMMMPL.mzn",
                "firstLine": 64,
                "firstColumn": 3,
                "lastLine": 64,
                "lastColumn": 100
            },
            "isCompIter": false,
            "description": "binary '->' operator expression"
        },
        {
            "location": {
                "filename": "/tmp/idesyde/minizinc/AADPMMMPL.mzn",
                "firstLine": 64,
                "firstColumn": 47,
                "lastLine": 64,
                "lastColumn": 100
            },
            "isCompIter": false,
            "description": "binary '\\/' operator expression"
        },
        {
            "location": {
                "filename": "/tmp/idesyde/minizinc/AADPMMMPL.mzn",
                "firstLine": 64,
                "firstColumn": 78,
                "lastLine": 64,
                "lastColumn": 100
            },
            "isCompIter": false,
            "description": "binary '!=' operator expression"
        },
        {
            "location": {
                "filename": "/tmp/idesyde/minizinc/AADPMMMPL.mzn",
                "firstLine": 64,
                "firstColumn": 78,
                "lastLine": 64,
                "lastColumn": 94
            },
            "isCompIter": false,
            "description": "array access"
        }
    ],
    "message": "undefined result becomes false in Boolean context\n  (array access out of bounds, array has index set 0..0, but given index is 1)"
}
Rojods commented 6 months ago

I made a silly mistake in the indexing of the Minizinc side within the decision model. I think the latest develop commit has fixed the issue! Could you check it out, please? :)

BeethovenKodar commented 6 months ago

I am currently pushing all my test cases (4 of them).

In general it seems like these test cases are never possible to evaluate hw mappings, which seems like an effect of changing something in recent commits.

Rojods commented 5 months ago

Gotcha, will take a look at them!

Rojods commented 5 months ago

Good news is that I already discovered why tc1 fails: the time to transfer data in the interconnects is gigantic, and my number scaling has not properly accounted for huge numbers in the interconnect, only in the computation and memories!

For tc3, I am still investigating. It complains of inconsistency without further info, so that's not nice, but we will manage.

Rojods commented 5 months ago

@BeethovenKodar I made some small breakthrough in the debugging here: it seems the greatest enemy we have now is that the FPGA element in the MPSoC_Final model cannot reach back any memory to write it there. So, there are a couple things that could have gone wrong here:

  1. The model is actually wrong in the sense that there is no directional interconnect path from a FPGA to a memory element.
  2. We messed up the path-constructing logic in the identification rule.

Could you please check out that 1) is not true, please? :)

BeethovenKodar commented 5 months ago

I just pushed a new mpsoc version ti the examples. The previous one was strange. See if this fixes it, I can check the specified connections in the meantime!

BeethovenKodar commented 5 months ago

There are three two-way connections from the FPGA, of which one connects to the PL DDR4 switch (memory switch). There is also some bram memory that only two-way connects to the FPGA, nowhere else.

Like, this used to work so the issue feels unrelated but who knows right.

Rojods commented 5 months ago

I guess it used to work because in some previous commits i was not enforcing that the mapping would 100% respect the paths in the interconnection. Now it does! So that could be the change that suddenly made us not have the solutions anymore. I'll try soon the new model and see!

BeethovenKodar commented 5 months ago

I realized my one-way connections had the wrong EdgeTrait, does it help with anything? I pushed the change anyhow.

Rojods commented 5 months ago

It did help! There was also another subtle mistake in the IRule identifying the new hardware model (MMCore...). Now the only failing case is tc3. BUT, I think the mistake is something a bit funnier: did you see that one of the actors actually requires mores BRAM than the FPGA provides? Is this in purpose? So that no solution exists? This same exact thing happens with the "Area" requirement of the other actor.

If you change this to some smaller zeroes, it works!

Rojods commented 5 months ago

I just added a feasible version of tc3 with this feasibility: tc3_feasible.

BeethovenKodar commented 5 months ago

Hahah yes it is meant to fail!!

image

Rojods commented 5 months ago

Well, we can accept crash as passing :P. I am not sure if it still crash or the latest commits just finishes with no solution. I might try to pretty it out later.

BeethovenKodar commented 5 months ago

Results are in for the test cases finally! No need to fix the crash as it is perfectly fine as there is a strong correlation to exceeding the limits.

Rojods commented 5 months ago

Since we seem to have fixed this issue for now, I will close it :).