search

StanfordAHA / CGRAFlow

Integration test for entire CGRA flow
BSD 3-Clause "New" or "Revised" License
12 stars 2 forks source link

Flow breaks when constant name changes in design_top.json #3

Closed steveri closed 7 years ago

steveri commented 7 years ago

When Jeff changes the name of the const from "const" to "const_186" the flow breaks. I (steveri) added a hack in the travis script to undo the name, but this is of course not the right fix. I suspect the problem is either in the mapper or in one of the pnr scripts, respectively. You can see source for working and non-working design_top.json in builds 145 (https://travis-ci.org/StanfordAHA/CGRAFlow/builds/219574814) and 146 (https://travis-ci.org/StanfordAHA/CGRAFlow/builds/219898992) respectively---the only difference is one line where the working version says "const" and the nonworking version says "const_186"

diff design_top_145.json design_top_146.json < "const"

"const_186"

jeffsetter commented 7 years ago

Note that "const_186" is simply a name of the constant being using. Ideally subsequent scripts would not rely on the instance name for parsing.

steveri commented 7 years ago

Yes; to be clear, this is exactly what we want to fix. Subsequent-script authors please verify that you do not rely on this name to always/only be "const"

On April 8, 2017 4:51:23 PM PDT, Jeff Setter notifications@github.com wrote:

Note that "const_186" is simply a name of the constant being using. Ideally subsequent scripts would not rely on the instance name for parsing.

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

jameshegarty commented 7 years ago

Remember Ross wrote the mapper, not me... I looked at his code, and he does hardcode a "const" in a few places, but it wasn't obvious to me that that was the problem. It could be. IMO @rdaly525 and @cdonovick should update their code to error out if the input doesn't match what is expected. As of right now, it looks like their code is silently failing, which makes this hard to track down.

I would maybe also recommend removing the workaround... keeping the build working is a good idea, but IMO this is a real problem we should track down. But thanks for doing this Steven!

Is everyone getting these alerts?

jameshegarty commented 7 years ago

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change.

steveri commented 7 years ago 
I tried the flow twice in quick succession, with the only difference
being constant name "const_186" (build 150, failed) vs. "const" 
(Build 151, passed) in "design_top.json", so I'm pretty sure that
new change is causing the problem downstream.  Also if you look at
past builds that succeed, all use "const" as the constant name.

If you want to do the experiment yourself, simply uncomment my
travis script hack...

  # FIXME/TODO oops Jeff's recent change broke the flow.
  # This temporary hack undoes the change and lets the
  script succeed,
    # but this is not a reasonable fix going forward!
  - mv build/design_top.json /tmp/design_top.json
  - sed 's/const_186/const/' /tmp/design_top.json >
  build/design_top.json
  - grep const build/design_top.json /tmp/design_top.json

...and you can see it fail again...

SR

PS FWIW it looks to me like the mapper script more or less passes
the constant name through unchanged, from "design_top.json" to
"mapped.json," so I suspect that it's in the PNR script where things
fall apart...

On 4/8/2017 5:31 PM, jameshegarty
  wrote:

  It's possible btw that this has nothing to do with Jeff's
    change, remember, we pull down all the repos each time so it
    could have been another change.
cdonovick commented 7 years ago

If changing the name of the constant changes value of instance.get_config_value('op') from 'const' to something else then it will break my code. However, as the API is now there is no way for me to work around that.

Caleb

On Apr 8, 2017 7:49 PM, "steveri" notifications@github.com wrote:

I tried the flow twice in quick succession, with the only difference being constant name "const_186" (build 150, failed) vs. "const" (Build 151, passed) in "design_top.json", so I'm pretty sure that new change is causing the problem downstream. Also if you look at past builds that succeed, all use "const" as the constant name.

If you want to do the experiment yourself, simply uncomment my travis script hack...

FIXME/TODO oops Jeff's recent change broke the flow.

This temporary hack undoes the change and lets the

script succeed,

but this is not a reasonable fix going forward!

  • mv build/design_top.json /tmp/design_top.json
  • sed 's/const_186/const/' /tmp/design_top.json > build/design_top.json
  • grep const build/design_top.json /tmp/design_top.json

...and you can see it fail again...

SR

PS FWIW it looks to me like the mapper script more or less passes the constant name through unchanged, from "design_top.json" to "mapped.json," so I suspect that it's in the PNR script where things fall apart...

On 4/8/2017 5:31 PM, jameshegarty wrote:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/StanfordAHA/CGRAFlow","title": "StanfordAHA/CGRAFlow","subtitle":"GitHub repository","main_image_url":" https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/StanfordAHA/CGRAFlow"}}," updates":{"snippets":[{"icon":"PERSON","message":"@jameshegarty in #3: It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change."}],"action":{"name":"View Issue","url":"https://github. com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292755048"}}}

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292759941, or mute the thread https://github.com/notifications/unsubscribe-auth/AF13dCCYgaJ7O0cIgi3gVqf8QZYIetbPks5ruEclgaJpZM4M33iU .

steveri commented 7 years ago

It appears to be producing this (see build log 149 https://travis-ci.org/StanfordAHA/CGRAFlow/builds/220134733):

instances": { "const_186": { "config": { "constvalue": 2, "op": "const"

On April 8, 2017 9:10:43 PM PDT, Caleb Donovick notifications@github.com wrote:

If changing the name of the constant changes value of instance.get_config_value('op') from 'const' to something else then it will break my code. However, as the API is now there is no way for me to work around that.

Caleb

On Apr 8, 2017 7:49 PM, "steveri" notifications@github.com wrote:

I tried the flow twice in quick succession, with the only difference being constant name "const_186" (build 150, failed) vs. "const" (Build 151, passed) in "design_top.json", so I'm pretty sure that new change is causing the problem downstream. Also if you look at past builds that succeed, all use "const" as the constant name.

If you want to do the experiment yourself, simply uncomment my travis script hack...

FIXME/TODO oops Jeff's recent change broke the flow.

This temporary hack undoes the change and lets the

script succeed,

but this is not a reasonable fix going forward!

  • mv build/design_top.json /tmp/design_top.json
  • sed 's/const_186/const/' /tmp/design_top.json > build/design_top.json
  • grep const build/design_top.json /tmp/design_top.json

...and you can see it fail again...

SR

PS FWIW it looks to me like the mapper script more or less passes the constant name through unchanged, from "design_top.json" to "mapped.json," so I suspect that it's in the PNR script where things fall apart...

On 4/8/2017 5:31 PM, jameshegarty wrote:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/StanfordAHA/CGRAFlow","title": "StanfordAHA/CGRAFlow","subtitle":"GitHub repository","main_image_url":"

https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/StanfordAHA/CGRAFlow"}}," updates":{"snippets":[{"icon":"PERSON","message":"@jameshegarty in

3:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change."}],"action":{"name":"View Issue","url":"https://github. com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292755048"}}}

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292759941, or mute the thread

https://github.com/notifications/unsubscribe-auth/AF13dCCYgaJ7O0cIgi3gVqf8QZYIetbPks5ruEclgaJpZM4M33iU .

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

steveri commented 7 years ago

Oh and also this, earlier in the same file

        "instances": {
          "const_186": {
            "config": {
              "value": 2
            },

On April 8, 2017 9:41:47 PM PDT, Stephen steveri@steveri.com wrote:

It appears to be producing this (see build log 149 https://travis-ci.org/StanfordAHA/CGRAFlow/builds/220134733):

instances": { "const_186": { "config": { "constvalue": 2, "op": "const"

On April 8, 2017 9:10:43 PM PDT, Caleb Donovick notifications@github.com wrote:

If changing the name of the constant changes value of instance.get_config_value('op') from 'const' to something else then it will break my code. However, as the API is now there is no way for me to work around that.

Caleb

On Apr 8, 2017 7:49 PM, "steveri" notifications@github.com wrote:

I tried the flow twice in quick succession, with the only difference being constant name "const_186" (build 150, failed) vs. "const" (Build 151, passed) in "design_top.json", so I'm pretty sure that new change is causing the problem downstream. Also if you look at past builds that succeed, all use "const" as the constant name.

If you want to do the experiment yourself, simply uncomment my travis script hack...

FIXME/TODO oops Jeff's recent change broke the flow.

This temporary hack undoes the change and lets the

script succeed,

but this is not a reasonable fix going forward!

  • mv build/design_top.json /tmp/design_top.json
  • sed 's/const_186/const/' /tmp/design_top.json > build/design_top.json
  • grep const build/design_top.json /tmp/design_top.json

...and you can see it fail again...

SR

PS FWIW it looks to me like the mapper script more or less passes the constant name through unchanged, from "design_top.json" to "mapped.json," so I suspect that it's in the PNR script where things fall apart...

On 4/8/2017 5:31 PM, jameshegarty wrote:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/StanfordAHA/CGRAFlow","title": "StanfordAHA/CGRAFlow","subtitle":"GitHub repository","main_image_url":"

https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/StanfordAHA/CGRAFlow"}}," updates":{"snippets":[{"icon":"PERSON","message":"@jameshegarty in

3:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change."}],"action":{"name":"View Issue","url":"https://github. com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292755048"}}}

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292759941, or mute the thread

https://github.com/notifications/unsubscribe-auth/AF13dCCYgaJ7O0cIgi3gVqf8QZYIetbPks5ruEclgaJpZM4M33iU .

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

cdonovick commented 7 years ago

The latter of the two ?might? break my code but I'm not completely up on COREIR format. Looked 150 like it broke because missed named filles.

Caleb

On Apr 8, 2017 9:47 PM, "steveri" notifications@github.com wrote:

Oh and also this, earlier in the same file

"instances": { "const_186": { "config": { "value": 2 },

On April 8, 2017 9:41:47 PM PDT, Stephen steveri@steveri.com wrote:

It appears to be producing this (see build log 149 https://travis-ci.org/StanfordAHA/CGRAFlow/builds/220134733):

instances": { "const_186": { "config": { "constvalue": 2, "op": "const"

On April 8, 2017 9:10:43 PM PDT, Caleb Donovick notifications@github.com wrote:

If changing the name of the constant changes value of instance.get_config_value('op') from 'const' to something else then it will break my code. However, as the API is now there is no way for me to work around that.

Caleb

On Apr 8, 2017 7:49 PM, "steveri" notifications@github.com wrote:

I tried the flow twice in quick succession, with the only difference being constant name "const_186" (build 150, failed) vs. "const" (Build 151, passed) in "design_top.json", so I'm pretty sure that new change is causing the problem downstream. Also if you look at past builds that succeed, all use "const" as the constant name.

If you want to do the experiment yourself, simply uncomment my travis script hack...

FIXME/TODO oops Jeff's recent change broke the flow.

This temporary hack undoes the change and lets the

script succeed,

but this is not a reasonable fix going forward!

  • mv build/design_top.json /tmp/design_top.json
  • sed 's/const_186/const/' /tmp/design_top.json > build/design_top.json
  • grep const build/design_top.json /tmp/design_top.json

...and you can see it fail again...

SR

PS FWIW it looks to me like the mapper script more or less passes the constant name through unchanged, from "design_top.json" to "mapped.json," so I suspect that it's in the PNR script where things fall apart...

On 4/8/2017 5:31 PM, jameshegarty wrote:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/StanfordAHA/CGRAFlow","title": "StanfordAHA/CGRAFlow","subtitle":"GitHub repository","main_image_url":"

https://cloud.githubusercontent.com/assets/ 143418/17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/StanfordAHA/CGRAFlow"}}," updates":{"snippets":[{"icon":"PERSON","message":"@jameshegarty in

3:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change."}],"action":{"name":"View Issue","url":"https://github. com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292755048"}}}

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

<https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292759941 , or mute the thread

https://github.com/notifications/unsubscribe-auth/ AF13dCCYgaJ7O0cIgi3gVqf8QZYIetbPks5ruEclgaJpZM4M33iU .

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292763903, or mute the thread https://github.com/notifications/unsubscribe-auth/AF13dFqW-N6Tc6EkA_WEHjpajPWCEXioks5ruGLDgaJpZM4M33iU .

cdonovick commented 7 years ago

I'll look at all of it tomorrow, trying to read Travis logs on a phone is suboptimal.

On Apr 8, 2017 9:49 PM, "Caleb Donovick" cdonovick@gmail.com wrote:

The latter of the two ?might? break my code but I'm not completely up on COREIR format. Looked 150 like it broke because missed named filles.

Caleb

On Apr 8, 2017 9:47 PM, "steveri" notifications@github.com wrote:

Oh and also this, earlier in the same file

"instances": { "const_186": { "config": { "value": 2 },

On April 8, 2017 9:41:47 PM PDT, Stephen steveri@steveri.com wrote:

It appears to be producing this (see build log 149 https://travis-ci.org/StanfordAHA/CGRAFlow/builds/220134733):

instances": { "const_186": { "config": { "constvalue": 2, "op": "const"

On April 8, 2017 9:10:43 PM PDT, Caleb Donovick notifications@github.com wrote:

If changing the name of the constant changes value of instance.get_config_value('op') from 'const' to something else then it will break my code. However, as the API is now there is no way for me to work around that.

Caleb

On Apr 8, 2017 7:49 PM, "steveri" notifications@github.com wrote:

I tried the flow twice in quick succession, with the only difference being constant name "const_186" (build 150, failed) vs. "const" (Build 151, passed) in "design_top.json", so I'm pretty sure that new change is causing the problem downstream. Also if you look at past builds that succeed, all use "const" as the constant name.

If you want to do the experiment yourself, simply uncomment my travis script hack...

FIXME/TODO oops Jeff's recent change broke the flow.

This temporary hack undoes the change and lets the

script succeed,

but this is not a reasonable fix going forward!

  • mv build/design_top.json /tmp/design_top.json
  • sed 's/const_186/const/' /tmp/design_top.json > build/design_top.json
  • grep const build/design_top.json /tmp/design_top.json

...and you can see it fail again...

SR

PS FWIW it looks to me like the mapper script more or less passes the constant name through unchanged, from "design_top.json" to "mapped.json," so I suspect that it's in the PNR script where things fall apart...

On 4/8/2017 5:31 PM, jameshegarty wrote:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/StanfordAHA/CGRAFlow","title": "StanfordAHA/CGRAFlow","subtitle":"GitHub repository","main_image_url":"

https://cloud.githubusercontent.com/assets/143418/ 17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/StanfordAHA/CGRAFlow"}}," updates":{"snippets":[{"icon":"PERSON","message":"@jameshegarty in

3:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change."}],"action":{"name":"View Issue","url":"https://github. com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292755048"}}}

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecom ment-292759941, or mute the thread

https://github.com/notifications/unsubscribe-auth/AF13dCC YgaJ7O0cIgi3gVqf8QZYIetbPks5ruEclgaJpZM4M33iU .

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292763903, or mute the thread https://github.com/notifications/unsubscribe-auth/AF13dFqW-N6Tc6EkA_WEHjpajPWCEXioks5ruGLDgaJpZM4M33iU .

steveri commented 7 years ago

Yes dont look at build 150 look at 149 instead.

On April 8, 2017 9:50:00 PM PDT, Caleb Donovick notifications@github.com wrote:

The latter of the two ?might? break my code but I'm not completely up on COREIR format. Looked 150 like it broke because missed named filles.

Caleb

On Apr 8, 2017 9:47 PM, "steveri" notifications@github.com wrote:

Oh and also this, earlier in the same file

"instances": { "const_186": { "config": { "value": 2 },

On April 8, 2017 9:41:47 PM PDT, Stephen steveri@steveri.com wrote:

It appears to be producing this (see build log 149 https://travis-ci.org/StanfordAHA/CGRAFlow/builds/220134733):

instances": { "const_186": { "config": { "constvalue": 2, "op": "const"

On April 8, 2017 9:10:43 PM PDT, Caleb Donovick notifications@github.com wrote:

If changing the name of the constant changes value of instance.get_config_value('op') from 'const' to something else then it will break my code. However, as the API is now there is no way for me to work around that.

Caleb

On Apr 8, 2017 7:49 PM, "steveri" notifications@github.com wrote:

I tried the flow twice in quick succession, with the only difference being constant name "const_186" (build 150, failed) vs. "const" (Build 151, passed) in "design_top.json", so I'm pretty sure that new change is causing the problem downstream. Also if you look at past builds that succeed, all use "const" as the constant name.

If you want to do the experiment yourself, simply uncomment my travis script hack...

FIXME/TODO oops Jeff's recent change broke the flow.

This temporary hack undoes the change and lets the

script succeed,

but this is not a reasonable fix going forward!

  • mv build/design_top.json /tmp/design_top.json
  • sed 's/const_186/const/' /tmp/design_top.json > build/design_top.json
  • grep const build/design_top.json /tmp/design_top.json

...and you can see it fail again...

SR

PS FWIW it looks to me like the mapper script more or less passes the constant name through unchanged, from "design_top.json" to "mapped.json," so I suspect that it's in the PNR script where things fall apart...

On 4/8/2017 5:31 PM, jameshegarty wrote:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/StanfordAHA/CGRAFlow","title": "StanfordAHA/CGRAFlow","subtitle":"GitHub repository","main_image_url":"

https://cloud.githubusercontent.com/assets/ 143418/17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/StanfordAHA/CGRAFlow"}}," updates":{"snippets":[{"icon":"PERSON","message":"@jameshegarty in

3:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change."}],"action":{"name":"View Issue","url":"https://github. com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292755048"}}}

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

<https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292759941 ,

or mute the thread

https://github.com/notifications/unsubscribe-auth/ AF13dCCYgaJ7O0cIgi3gVqf8QZYIetbPks5ruEclgaJpZM4M33iU .

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292763903, or mute the thread

https://github.com/notifications/unsubscribe-auth/AF13dFqW-N6Tc6EkA_WEHjpajPWCEXioks5ruGLDgaJpZM4M33iU .

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

steveri commented 7 years ago

Agreed, me too, my eyes are starting to tear up. :)

Certainly doesnt have to be fixed tonight.

SR

On April 8, 2017 9:51:37 PM PDT, Caleb Donovick notifications@github.com wrote:

I'll look at all of it tomorrow, trying to read Travis logs on a phone is suboptimal.

On Apr 8, 2017 9:49 PM, "Caleb Donovick" cdonovick@gmail.com wrote:

The latter of the two ?might? break my code but I'm not completely up on COREIR format. Looked 150 like it broke because missed named filles.

Caleb

On Apr 8, 2017 9:47 PM, "steveri" notifications@github.com wrote:

Oh and also this, earlier in the same file

"instances": { "const_186": { "config": { "value": 2 },

On April 8, 2017 9:41:47 PM PDT, Stephen steveri@steveri.com wrote:

It appears to be producing this (see build log 149 https://travis-ci.org/StanfordAHA/CGRAFlow/builds/220134733):

instances": { "const_186": { "config": { "constvalue": 2, "op": "const"

On April 8, 2017 9:10:43 PM PDT, Caleb Donovick notifications@github.com wrote:

If changing the name of the constant changes value of instance.get_config_value('op') from 'const' to something else then it will break my code. However, as the API is now there is no way for me to work around that.

Caleb

On Apr 8, 2017 7:49 PM, "steveri" notifications@github.com wrote:

I tried the flow twice in quick succession, with the only difference being constant name "const_186" (build 150, failed) vs. "const" (Build 151, passed) in "design_top.json", so I'm pretty sure that new change is causing the problem downstream. Also if you look at past builds that succeed, all use "const" as the constant name.

If you want to do the experiment yourself, simply uncomment my travis script hack...

FIXME/TODO oops Jeff's recent change broke the flow.

This temporary hack undoes the change and lets the

script succeed,

but this is not a reasonable fix going forward!

  • mv build/design_top.json /tmp/design_top.json
  • sed 's/const_186/const/' /tmp/design_top.json > build/design_top.json
  • grep const build/design_top.json /tmp/design_top.json

...and you can see it fail again...

SR

PS FWIW it looks to me like the mapper script more or less passes the constant name through unchanged, from "design_top.json" to "mapped.json," so I suspect that it's in the PNR script where things fall apart...

On 4/8/2017 5:31 PM, jameshegarty wrote:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/StanfordAHA/CGRAFlow","title": "StanfordAHA/CGRAFlow","subtitle":"GitHub repository","main_image_url":"

https://cloud.githubusercontent.com/assets/143418/ 17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/StanfordAHA/CGRAFlow"}}," updates":{"snippets":[{"icon":"PERSON","message":"@jameshegarty in

3:

It's possible btw that this has nothing to do with Jeff's change, remember, we pull down all the repos each time so it could have been another change."}],"action":{"name":"View Issue","url":"https://github. com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292755048"}}}

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecom ment-292759941, or mute the thread

https://github.com/notifications/unsubscribe-auth/AF13dCC YgaJ7O0cIgi3gVqf8QZYIetbPks5ruEclgaJpZM4M33iU .

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

https://github.com/StanfordAHA/CGRAFlow/issues/3#issuecomment-292763903, or mute the thread

https://github.com/notifications/unsubscribe-auth/AF13dFqW-N6Tc6EkA_WEHjpajPWCEXioks5ruGLDgaJpZM4M33iU .

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

steveri commented 7 years ago 
FYI I removed the log for build 150 so as to (hopefully) lessen
confusion (it broke because I initially messed up my sed-script
hack).  For an example of the failing design_top and mapped.json
files, see log 149 instead...
steveri commented 7 years ago 
For completeness, note, this (below) should have referred to "build
149, failed" and not "build 150, failed."  Build 150 is not
useful and I deleted it as per previous e-mail...

SR

On 4/8/2017 7:49 PM, Stephen Richardson
  wrote:

  I tried the flow twice in quick succession, with the only difference being constant name "const_186" (build 150, failed) vs. "const"  (Build 151, passed) in "design_top.json", so I'm pretty sure that new change is causing the problem downstream.  Also if you look at past builds that succeed, all use "const" as the constant name.

  If you want to do the experiment yourself, simply uncomment my
  travis script hack...

    # FIXME/TODO oops Jeff's recent change broke the flow.
    # This temporary hack undoes the change and lets the
    script succeed,
      # but this is not a reasonable fix going forward!
    - mv build/design_top.json /tmp/design_top.json
    - sed 's/const_186/const/' /tmp/design_top.json >
    build/design_top.json
    - grep const build/design_top.json /tmp/design_top.json

  ...and you can see it fail again...

  SR

  PS FWIW it looks to me like the mapper script more or less passes
  the constant name through unchanged, from "design_top.json" to
  "mapped.json," so I suspect that it's in the PNR script where
  things fall apart...
steveri commented 7 years ago

As recommended earlier by James, I am going to remove my sed hack from the travis script so that it will fail until we get this fixed...

I would maybe also recommend removing the workaround... keeping the build working is a good idea, but IMO this is a real problem we should track down.

steveri commented 7 years ago

BTW if (like me) you are annoyed that everyone can see your comments except you, note that there is a place in your account settings where you can set "include your own updates" (seems to be off by default) - see https://github.com/settings/notifications

rdaly525 commented 7 years ago

I am going to push a change that should fix this problem, but break Jeff's code. @jeffsetter I am changing the type of constant, so please update your code to take this into account. Previous: Array(16,BitOut) New: Record({"out",Array(16,BitOut)})

jeffsetter commented 7 years ago

I pushed the necessary change to my repo. It appears that it is working again.

jameshegarty commented 7 years ago

Great!