search

uwsampa / accept

an approximate compiler
http://accept.rocks
MIT License
37 stars 14 forks source link

The great NPU merge #23

Closed sampsyo closed 10 years ago

sampsyo commented 10 years ago

As of 00a84b7, we're beginning to merge the NPU branch back in. Lots of stuff left to do:

Totally deferrable style cleanup for later:

sampsyo commented 10 years ago

Clang has been fixed to emit metadata for function return values. We no longer need another file-on-the-side for this.

sampsyo commented 10 years ago

It occurs to me to note that the merged version does not yet treat parameter stores as approximate (since this is sound only for function regions!).

sampsyo commented 10 years ago

Here's where the merge currently stands for the benchmarks in the repo:

sampsyo commented 10 years ago

If you see this, @andreolb, are there other benchmarks I should be trying, or is it just inversek2j, jmeint, and sobel for now?

andreolb commented 10 years ago

Yes, only these. But I'm still wondering what do you mean by "mysteries" haha

On Tue, Jul 15, 2014 at 7:06 PM, Adrian Sampson notifications@github.com wrote:

If you see this, @andreolb https://github.com/andreolb, are there other benchmarks I should be trying, or is it just inversek2j, jmeint, and sobel for now?

— Reply to this email directly or view it on GitHub https://github.com/uwsampa/accept/issues/23#issuecomment-49115798.

André Oliveira

sampsyo commented 10 years ago

Oh yeah, sorry. :smiley: That was mainly a note to myself. Those are regions identified by the pass but then discarded for a reason I don't yet know. I need to add in more ACCEPT logging to explain them.

sampsyo commented 10 years ago

Oh, and thanks for clarifying! Here's hoping I have the right regions (and jmeint is easy to fix up).

sampsyo commented 10 years ago

Oh, BTW, Andre: if you're around tomorrow, maybe I can show you how the merged version works! It should make it real easy to, e.g., turn on and off NPUization without touching the code. And maybe we can converge future work, too. :sparkles: :rocket:

andreolb commented 10 years ago

Thanks Adrian! And I see you're really into emoticons now haha (I love them as well LOL)

Would you mind/get super-pissed/etc. if I keep using my setup a little bit longer? It's just because I'm still sorting some things out (like the performance issue) with Thierry and I'd rather rely on my ugly but familiar setup until these small details are hammered out... No hard feelings?

On Tue, Jul 15, 2014 at 8:37 PM, Adrian Sampson notifications@github.com wrote:

Oh, BTW, Andre: if you're around tomorrow, maybe I can show you how the merged version works! It should make it real easy to, e.g., turn on and off NPUization without touching the code. And maybe we can converge future work, too. [image: :sparkles:][image: :rocket:]

— Reply to this email directly or view it on GitHub https://github.com/uwsampa/accept/issues/23#issuecomment-49120311.

André Oliveira

sampsyo commented 10 years ago

Of course, go ahead. But as a warning, if you make more changes now, they might be somewhat difficult to merge back in the future, so it might be a good idea to switch over if you're going to make any substantial changes. Let me know whenever you want an introduction—it should not be hard to grok since I really only added stuff.

andreolb commented 10 years ago

Yeah sure! I don't foresee major changes coming anymore so no problem :)

On Tue, Jul 15, 2014 at 8:46 PM, Adrian Sampson notifications@github.com wrote:

Of course, go ahead. But as a warning, if you make more changes now, they might be somewhat difficult to merge back in the future, so it might be a good idea to switch over if you're going to make any substantial changes. Let me know whenever you want an introduction—it should not be hard to grok since I really only added stuff.

— Reply to this email directly or view it on GitHub https://github.com/uwsampa/accept/issues/23#issuecomment-49120742.

André Oliveira

sampsyo commented 10 years ago

Mysteries solved. Those regions are being discarded because no suitable precise-pure call can be found.

I believe we're all set for inversek2j and sobel. Ignoring jmeint for now due to low accuracy.

sampsyo commented 10 years ago

Okay, buffer size is now configurable. I'm calling the merge complete. Next step is to run experiments.

sampsyo commented 10 years ago

Experiments status as of the beginning of the talk this afternoon:

sampsyo commented 10 years ago

Thanks to @andreolb, the status is now:

andreolb commented 10 years ago

Adrian, could you try running sobel with a buffer size of 1152, please?

On Wed, Jul 16, 2014 at 8:12 PM, Adrian Sampson notifications@github.com wrote:

Thanks to @andreolb https://github.com/andreolb, the status is now:

  • inversek2j is totally working!
  • sobel is outputting strange, large integers (all in the ballpark of 52736 where they should be 0-255)

— Reply to this email directly or view it on GitHub https://github.com/uwsampa/accept/issues/23#issuecomment-49254111.

André Oliveira

sampsyo commented 10 years ago

Alas, I get about the same output. It looks something like this:

256,256
57856,57856,57856,57344,57344,57344,57600,57600,...

It could be that something's wrong on the source side… I'll keep investigating.

sampsyo commented 10 years ago

Wait, I may not have done that execution correctly. I'll try it again.

sampsyo commented 10 years ago

Yeah, still exactly the same output. (Sorry for the email noise.) Not really sure what's up.

andreolb commented 10 years ago

Sure, no problem! What's the input image? (just the file name :)

On Wed, Jul 16, 2014 at 8:24 PM, Adrian Sampson notifications@github.com wrote:

Yeah, still exactly the same output. (Sorry for the email noise.) Not really sure what's up.

— Reply to this email directly or view it on GitHub https://github.com/uwsampa/accept/issues/23#issuecomment-49254636.

André Oliveira

sampsyo commented 10 years ago

It's lena.dat—Thierry mentioned that those dimensions worked the best.

andreolb commented 10 years ago

Yes, that's correct. I'll investigate this!

On Wed, Jul 16, 2014 at 8:27 PM, Adrian Sampson notifications@github.com wrote:

It's lena.dat—Thierry mentioned that those dimensions worked the best.

— Reply to this email directly or view it on GitHub https://github.com/uwsampa/accept/issues/23#issuecomment-49254753.

André Oliveira

sampsyo commented 10 years ago

Okay, cool! I'll poke around too. Hopefully we won't collide trying to use zynq2. :smiley:

andreolb commented 10 years ago

haha we won't. I'll try to solve this without using zynq2 hahahahaha

On Wed, Jul 16, 2014 at 8:31 PM, Adrian Sampson notifications@github.com wrote:

Okay, cool! I'll poke around too. Hopefully we won't collide trying to use zynq2. [image: :smiley:]

— Reply to this email directly or view it on GitHub https://github.com/uwsampa/accept/issues/23#issuecomment-49254915.

André Oliveira

andreolb commented 10 years ago

Oh, I forgot to mention. Buffer size must be 1152 for sobel, so keep that change :)

On Wed, Jul 16, 2014 at 8:31 PM, Adrian Sampson notifications@github.com wrote:

Okay, cool! I'll poke around too. Hopefully we won't collide trying to use zynq2. [image: :smiley:]

— Reply to this email directly or view it on GitHub https://github.com/uwsampa/accept/issues/23#issuecomment-49254915.

André Oliveira

sampsyo commented 10 years ago

Okay, buffer sizes committed (above). I'm off for a little bit to bike home, so zynq2 is yours if you want it for a while anyway. :sparkles: