Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Num trait to Complex #421

Closed
wants to merge 6 commits into from
Closed

Add Num trait to Complex #421

wants to merge 6 commits into from

Conversation

grebe
Copy link
Contributor

@grebe grebe commented May 13, 2015

There are a few other fixes that got included here, but the main point is to add the Num trait to Complex.

It is probably worth discussing whether Complex is well suited to the Num trait- I imagine when it was first written, it was thought to be a bad idea. What I want is to be able to write DSP blocks with generics that don't care whether the underlying types are real, complex, or perhaps other objects like vectors or quaternions. What we currently have is a "one size fits all" Num trait, so what I'm proposing now is adding not-quite-right implementations for the things that don't make sense, notably comparison and modulus. Perhaps it would be better to leave them as ???.

I also added a protected function to checkPort to the Bundle class. When calcElements runs, it uses reflection to go through each method and tries to invoke it. Unfortunately, for Complex, some of those methods construct a new Complex, which can lead to a stack overflow. The new function checkPort is supposed to fix this by allowing subclasses of Bundle to filter out elements before invocation. This is related to Albert's old pull request. Adding checkPort is also nice if you want to restrict what types of objects can be in a subclass of Bundle. For the SDR project, I have subclasses of Bundle that throw a ChiselError if any of their objects don't conform to our ready/valid interface.

It may be worth thinking about splitting up Num into a numerical tower that makes sense for DSP applications. This page has a bunch of links, this page discusses some of the problems with Haskell's numeric types (it would be nice to avoid these). Some things that seem essential to DSP are:

  • Where possible, make it easy to write modules that work for both real and complex numbers
  • Get easy access to additive (and multiplicative) identity
  • Mixing types should be easy, i.e. multiplying a SInt by a SFix should do the right thing.

@sdtwigg
Copy link
Contributor

sdtwigg commented May 14, 2015

The checkPort trick is an interesting alternative to the dummy variable
trick for ensuring functions are not improperly interpreted as
data-carrying members of a Bundle.
On May 13, 2015 3:29 PM, "grebe" [email protected] wrote:

There are a few other fixes that got included here, but the main point is
to add the Num trait to Complex.

It is probably worth discussing whether Complex is well suited to the Num
trait- I imagine when it was first written, it was thought to be a bad
idea. What I want is to be able to write DSP blocks with generics that
don't care whether the underlying types are real, complex, or perhaps other
objects like vectors or quaternions. What we currently have is a "one size
fits all" Num trait, so what I'm proposing now is adding not-quite-right
implementations for the things that don't make sense, notably comparison
and modulus. Perhaps it would be better to leave them as ???.

I also added a protected function to checkPort to the Bundle class. When
calcElements runs, it uses reflection to go through each method and tries
to invoke it. Unfortunately, for Complex, some of those methods construct a
new Complex, which can lead to a stack overflow. The new function checkPort
is supposed to fix this by allowing subclasses of Bundle to filter out
elements before invocation. This is related to Albert's old pull request
#181. Adding checkPort is also
nice if you want to restrict what types of objects can be in a subclass of
Bundle. For the SDR project, I have subclasses of Bundle that throw a
ChiselError if any of their objects don't conform to our ready/valid
interface.

It may be worth thinking about splitting up Num into a numerical tower
that makes sense for DSP applications. This page
rust-lang/rust#4231 has a bunch of links, this
page https://ghc.haskell.org/trac/haskell-prime/wiki/NumericClasses
discusses some of the problems with Haskell's numeric types (it would be
nice to avoid these). Some things that seem essential to DSP are:

  • Where possible, make it easy to write modules that work for both
    real and complex numbers
  • Get easy access to additive (and multiplicative) identity
  • Mixing types should be easy, i.e. multiplying a SInt by a SFix
    should do the right thing.

You can view, comment on, or merge this pull request online at:

#421
Commit Summary

  • Make Complex have Num trait, introduce conjugate.
  • Add a hook to calcElements in Bundle to let subclasses of Bundle
  • Fix null pointer exception for nodes with no component.
  • Filter out all but real and imag from Complex bundle elements
  • Fix stack overflow when computing elements in bundle causes methods
    like
  • Add f< and other missing operations to Cpp backend.

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#421.

@ucbjrl
Copy link
Contributor

ucbjrl commented May 14, 2015

I've added some test code and created the branch complex421.

@ucbjrl ucbjrl closed this May 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants