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

#1 Allow more heterogenous types in numeric operations #36

Closed

Conversation

julienrf
Copy link
Contributor

@julienrf julienrf commented Oct 4, 2012

Hi,

Here is another attempt to resolve this issue. I’m using a pattern like CanBuildFrom to express what the return type of a numeric operation should be. That allows to express type promotion (e.g. Int to Float) in a clean way.

Unfortunately I’m unable to make things like 1 + unit(1) working… The anyToNumericOps implicit conversion does not seem to be applied automatically.

@TiarkRompf
Copy link
Owner

I think we'll need to figure out how to make things like 1 + unit(1) work reliably, too. I wonder whether part of the problem is that we've been trying to be too generic all along. Maybe it's worth considering a version that has explicit cases for all primitive types, similar to this one: https://github.com/TiarkRompf/graal-playground/blob/master/src/main/scala/playground/interpreter/Core.scala

@julienrf
Copy link
Contributor Author

Yes I understand but that would be less nice. I’d like to avoid this kind of code duplication.
Actually I’m wondering if there’s a bug in the Scala compiler since even the following implementation doesn’t work:

  implicit class NumericOpsCls[A : Numeric : Manifest](lhs: A) {
    def + [B, C](rhs: Rep[B])(implicit op: (A ~ B := C), mC: Manifest[C], sc: SourceContext) = numeric_plus(op.lhs(unit(lhs)), op.rhs(rhs))(op.Numeric, mC, sc)
  }

Note that the implicit conversion operates on a A rather than on a Rep[A], so if I write 1 + unit(1) the compiler should see that none of the overloads of Int#+ works with a Rep[Int], so it should try to look for implicit conversions defining a method + on a Rep[Int] and there is such a conversion (NumericOpsCls) but it needs Numeric[Int] and Manifest[Int] instances to be applicable, these values are available so the implicit conversion should be applicable provided there is an implicit value of type A ~ B := C where A and B are fixed to Int, and there is such an implicit value (numericSameArgs, of type A ~ A := A) so it should fix the C type parameter to Int and go on.
Why is it not working as described?

@TiarkRompf
Copy link
Owner

I agree the explicit version would be less nice. I'm not 100% sure where this one goes wrong (you can try running scalac with -Ytyper-debug to find out more) but it seems like one problem here is guessing C.

Since the A ~ B := C objects essentially encode numeric LUBs, maybe it would work to make those explicit, i.e. provide implicit instances for Int ~ Int := Int, Int ~ Double := Double etc.

@julienrf
Copy link
Contributor Author

Since the A ~ B := C objects essentially encode numeric LUBs, maybe it would work to make those explicit, i.e. provide implicit instances for Int ~ Int := Int, Int ~ Double := Double etc.

It didn’t work either.

I could only get it working by making the + method monomorphic (so it works only with operands of the same types):

implicit class NumericOpsCls[A : Numeric : Manifest](lhs: A) {
  def + (rhs: Rep[A])(implicit op: (A ~ A := A), sc: SourceContext) = numeric_plus(op.lhs(unit(lhs)), op.rhs(rhs))(op.Numeric, manifest[A], sc)
}

But I still doesn’t understand why the polymorphic way works well when the left hand side is a Rep[Int] but doesn’t when it’s an Int. Do you think someone else like e.g. gkossakowski could help us? (although I know the Scala team is very busy currently)

@kjbrown kjbrown closed this Apr 12, 2013
@julienrf
Copy link
Contributor Author

Hi, thanks @kjbrown for cleaning things up, but why has this issue been closed? I think we really need to find a solution.

@kjbrown
Copy link
Collaborator

kjbrown commented Apr 12, 2013

I agree... it must have closed automatically when I deleted delite-develop-M7. There is another instance of this issue still open though.

@xeno-by
Copy link

xeno-by commented Aug 22, 2013

@julienrf Could the problem you were facing be an instance of scala/scala#2822?

@soc
Copy link

soc commented Aug 23, 2013

Reminds me more of https://issues.scala-lang.org/browse/SI-5107 and another issue I can't find in JIRA. There are multiple things which come together here which prevent this code from working.

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.

5 participants