Convolution basic implementation #115

ktakagaki · 2014-01-09T20:51:27Z

Hi David et al.,

I am working on programming in basic FIR filter capabilities, and have finished the basics of a convolve() function for DenseVector[Double], with unit tests. I need input on option handling before I get too deep in filling out the code...

I've implemented an overhang option like this, with an option object:
def convolve[Input, Output](kernel: Input, data: Input, overhangOpt: OverhangOpt = OverhangOpt.Default()) (implicit canConvolve: CanConvolve[Input, Output]): Output

abstract class OverhangOpt object OverhangOpt{ case class Default() extends OverhangOpt case class Sequence(k: (Integer, Integer)) extends OverhangOpt case class Integer(k: Integer) extends OverhangOpt }

Is this along the lines of what you meant, David, when you were discussing how to handle options?
(https://groups.google.com/forum/#!topic/scala-breeze/o7A49ZYP1kg)

dlwh · 2014-01-11T18:40:07Z

Yeah, I think this is exactly right. Also, could you look into the test
failure for the RandomAccessFile stuff (it showed up on this build):
https://travis-ci.org/scalanlp/breeze/jobs/16680369

Thanks!

-- David

On Thu, Jan 9, 2014 at 12:51 PM, ktakagaki notifications@github.com wrote:

Hi David et al.,

I am working on programming in basic FIR filter capabilities, and have
finished the basics of a convolve() function for DenseVector[Double], with
unit tests. I need input on option handling before I get too deep in
filling out the code...

I've implemented an overhang option like this, with an option object:
def convolve[Input, Output](kernel: Input, data: Input, overhangOpt:
OverhangOpt = OverhangOpt.Default%28%29)
(implicit canConvolve: CanConvolve[Input, Output]): Output

abstract class OverhangOpt
object OverhangOpt{
case class Default() extends OverhangOpt
case class Sequence(k: (Integer, Integer)) extends OverhangOpt
case class Integer(k: Integer) extends OverhangOpt
}

Is this along the lines of what you meant, David, when you were discussing
how to handle options?

(https://groups.google.com/forum/#!topic/scala-breeze/o7A49ZYP1kg)

You can merge this Pull Request by running

git pull https://github.com/ktakagaki/breeze master

Or view, comment on, or merge it at:

#115
Commit Summary

Convolution test implementation

File Changes

M src/main/scala/breeze/signal/package.scalahttps://github.com/Convolution basic implementation #115/files#diff-0(25)

A src/main/scala/breeze/signal/support/CanConvlolve.scalahttps://github.com/Convolution basic implementation #115/files#diff-1(84)

M src/test/scala/breeze/signal/SignalTest.scalahttps://github.com/Convolution basic implementation #115/files#diff-2(30)

Patch Links:

https://github.com/scalanlp/breeze/pull/115.patch

https://github.com/scalanlp/breeze/pull/115.diff

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/115
.

dlwh · 2014-01-11T18:40:57Z

well, maybe just make Sequence take two ints rather than a pair of ints?
Also, I think think you meant for them to take "Integers", but probably
Ints?

-- David

On Sat, Jan 11, 2014 at 10:40 AM, David Hall dlwh@cs.berkeley.edu wrote:

Yeah, I think this is exactly right. Also, could you look into the test
failure for the RandomAccessFile stuff (it showed up on this build):
https://travis-ci.org/scalanlp/breeze/jobs/16680369

Thanks!

-- David

On Thu, Jan 9, 2014 at 12:51 PM, ktakagaki notifications@github.comwrote:

Hi David et al.,

I am working on programming in basic FIR filter capabilities, and have
finished the basics of a convolve() function for DenseVector[Double], with
unit tests. I need input on option handling before I get too deep in
filling out the code...

I've implemented an overhang option like this, with an option object:
def convolve[Input, Output](kernel: Input, data: Input, overhangOpt:
OverhangOpt = OverhangOpt.Default%28%29)
(implicit canConvolve: CanConvolve[Input, Output]): Output

abstract class OverhangOpt
object OverhangOpt{
case class Default() extends OverhangOpt
case class Sequence(k: (Integer, Integer)) extends OverhangOpt
case class Integer(k: Integer) extends OverhangOpt
}

Is this along the lines of what you meant, David, when you were
discussing how to handle options?

(https://groups.google.com/forum/#!topic/scala-breeze/o7A49ZYP1kg)

You can merge this Pull Request by running

git pull https://github.com/ktakagaki/breeze master

Or view, comment on, or merge it at:

#115
Commit Summary

Convolution test implementation

File Changes

M src/main/scala/breeze/signal/package.scalahttps://github.com/Convolution basic implementation #115/files#diff-0(25)

A src/main/scala/breeze/signal/support/CanConvlolve.scalahttps://github.com/Convolution basic implementation #115/files#diff-1(84)

M src/test/scala/breeze/signal/SignalTest.scalahttps://github.com/Convolution basic implementation #115/files#diff-2(30)

Patch Links:

https://github.com/scalanlp/breeze/pull/115.patch

https://github.com/scalanlp/breeze/pull/115.diff

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/115
.

ktakagaki · 2014-01-13T09:10:25Z

OK regarding the Ints.

Regarding the Travis builds, the same tests are passing just fine on my
system, and on code review, I can't think of a reason why these two
specific functions/tests are failing and not others...
Google isn't helping either, do you have any ideas? I will keep thinking,
but I'm kind of stuck...

I've inserted some println()s in the test function to troubleshoot what's
happening on the Travis server, and filed a pull request, maybe that will
clarify...?
#115

Kenta

On Sat, Jan 11, 2014 at 7:41 PM, David Hall notifications@github.comwrote:

well, maybe just make Sequence take two ints rather than a pair of ints?
Also, I think think you meant for them to take "Integers", but probably
Ints?

-- David

On Sat, Jan 11, 2014 at 10:40 AM, David Hall dlwh@cs.berkeley.edu
wrote:

Yeah, I think this is exactly right. Also, could you look into the test
failure for the RandomAccessFile stuff (it showed up on this build):
https://travis-ci.org/scalanlp/breeze/jobs/16680369

Thanks!

-- David

On Thu, Jan 9, 2014 at 12:51 PM, ktakagaki notifications@github.comwrote:

Hi David et al.,

I am working on programming in basic FIR filter capabilities, and have
finished the basics of a convolve() function for DenseVector[Double],
with
unit tests. I need input on option handling before I get too deep in
filling out the code...

I've implemented an overhang option like this, with an option object:
def convolve[Input, Output](kernel: Input, data: Input, overhangOpt:
OverhangOpt = OverhangOpt.Default%28%29)
(implicit canConvolve: CanConvolve[Input, Output]): Output

abstract class OverhangOpt
object OverhangOpt{
case class Default() extends OverhangOpt
case class Sequence(k: (Integer, Integer)) extends OverhangOpt
case class Integer(k: Integer) extends OverhangOpt
}

Is this along the lines of what you meant, David, when you were
discussing how to handle options?

(https://groups.google.com/forum/#!topic/scala-breeze/o7A49ZYP1kg)

You can merge this Pull Request by running

git pull https://github.com/ktakagaki/breeze master

Or view, comment on, or merge it at:

#115
Commit Summary

Convolution test implementation

File Changes

M src/main/scala/breeze/signal/package.scala<
https://github.com/scalanlp/breeze/pull/115/files#diff-0>(25)

A src/main/scala/breeze/signal/support/CanConvlolve.scala<
https://github.com/scalanlp/breeze/pull/115/files#diff-1>(84)

M src/test/scala/breeze/signal/SignalTest.scala<
https://github.com/scalanlp/breeze/pull/115/files#diff-2>(30)

Patch Links:

https://github.com/scalanlp/breeze/pull/115.patch

https://github.com/scalanlp/breeze/pull/115.diff

—
Reply to this email directly or view it on GitHub<
https://github.com/scalanlp/breeze/pull/115>
.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/115#issuecomment-32103482
.

…d by Travis, solved by committing temp.bin to the repository.

ktakagaki · 2014-01-13T12:33:15Z

OK. I think I've solved the issue, it was that the Travis server was automatically deleting the temporary file "temp.bin" used for testing the binary writing functions. Since the delete was at indeterminate time, different tests were failing on different runs. The issue was solved by committing the temporary file "temp.bin" to the resources folder, I guess this flags it as something that the Travis server shouldn't touch, or something like that.

dlwh · 2014-01-14T17:46:34Z

Thanks!

On Mon, Jan 13, 2014 at 4:33 AM, ktakagaki notifications@github.com wrote:

OK. I think I've solved the issue, it was that the Travis server was
automatically deleting the temporary file "temp.bin" used for testing the
binary writing functions. Since the delete was at indeterminate time,
different tests were failing on different runs. The issue was solved by
committing the temporary file "temp.bin" to the resources folder, I guess
this flags it as something that the Travis server shouldn't touch, or
something like that.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/115#issuecomment-32165228
.

Convolution basic implementation

Convolution test implementation

a4d8f49

Travis CI build failing? Inserted println() to troubleshoot

5425706

ktakagaki added 4 commits January 13, 2014 11:47

Further troubleshooting with println

5e67e4b

Further testing

1b10cee

Add temp.bin to resources folder

d0902e8

Clean up after troubleshooting: issue was temporary file being delete…

b51a78b

…d by Travis, solved by committing temp.bin to the repository.

dlwh added a commit that referenced this pull request Jan 14, 2014

Merge pull request #115 from ktakagaki/master

d6dc13d

Convolution basic implementation

dlwh merged commit d6dc13d into scalanlp:master Jan 14, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convolution basic implementation #115

Convolution basic implementation #115

ktakagaki commented Jan 9, 2014

dlwh commented Jan 11, 2014

(https://groups.google.com/forum/#!topic/scala-breeze/o7A49ZYP1kg)

dlwh commented Jan 11, 2014

(https://groups.google.com/forum/#!topic/scala-breeze/o7A49ZYP1kg)

ktakagaki commented Jan 13, 2014

(https://groups.google.com/forum/#!topic/scala-breeze/o7A49ZYP1kg)

ktakagaki commented Jan 13, 2014

dlwh commented Jan 14, 2014

Convolution basic implementation #115

Convolution basic implementation #115

Conversation

ktakagaki commented Jan 9, 2014

dlwh commented Jan 11, 2014

(https://groups.google.com/forum/#!topic/scala-breeze/o7A49ZYP1kg)

dlwh commented Jan 11, 2014

(https://groups.google.com/forum/#!topic/scala-breeze/o7A49ZYP1kg)

ktakagaki commented Jan 13, 2014

(https://groups.google.com/forum/#!topic/scala-breeze/o7A49ZYP1kg)

ktakagaki commented Jan 13, 2014

dlwh commented Jan 14, 2014