algebra icon indicating copy to clipboard operation
algebra copied to clipboard

Semigroup/Monoid.combineN: Express partiality with values not exceptions

Open jbgi opened this issue 10 years ago • 6 comments

After reading http://plastic-idolatry.com/erik/sw2015.pdf it looks like Semigroup.combineN and Monoid.combineN currently only meet goal number 7 (Pragmatic) but not goal number 2 (Safe: Express partiality with values not exceptions).

  /**
   * Return `a` combined with itself `n` times.
   */
  def combineN(a: A, n: Int): A =
    if (n <= 0) throw new IllegalArgumentException("Repeated combining for semigroups must have n > 0")
    else repeatedCombineN(a, n)

jbgi avatar Sep 24 '15 18:09 jbgi

yes. That's right. Since returning Option[A] would require boxing, that was done. Shame we don't have some kind of non-negative integer type.

@non might want to comment. That method does not come up much at all as far as I know (I don't think I've ever used it in Twitter's code in Algebird, though we have an equivalent version).

We could put combineN in Group and combineNOption in Semigroup.

johnynek avatar Sep 24 '15 18:09 johnynek

It's true.

Other places you'll notice this problem are Field[A]#div (which returns A not Option[A]) and the pow operator as well (and sqrt/nroot in Spire).

non avatar Sep 24 '15 18:09 non

I think the current API is done since it is total for Group. In this case some kind of combineNOption makes sense -- but I think that the partiality guidelines will probably have to give in the face of e.g. division.

What I really want is a version which requires a literal Int whose sign can be checked at compile-time. (We could then use the thing to make safer versions of e.g. pow.)

non avatar Sep 24 '15 18:09 non

@non See https://github.com/non/spire/pull/438 - we can get unboxed (value class) + optionally checked at compile time for constants.

Specifically: https://github.com/kevinmeredith/spire/blob/jetdim_compile_time_check/macros/src/main/scala/spire/macros/PositiveInt.scala

(It's a case class there, but a value class may make more sense)

tixxit avatar Sep 24 '15 19:09 tixxit

PositiveInt is cool, maybe we should have something like that (also non-negative int might be good).

As an aside, we are likely to have partiality with div as Erik mentioned (and probably others). I wonder if we could create an annotation for such methods which might make it easier to test (no function should throw no matter what the inputs unless it has such an annotation).

You could imagine a macro that would prevent you from calling such methods inside the block (like a safe region), and we could add the total versions (which just lift the partiality into the return type).

It would be cool to have some trick with a method with a certain annotation could only be called INSIDE a given macro, like Rust's unsafe block. Emulating that in scala with macros + annotations would be awesome. I don't really see how to do it though (maybe with annotation macros if they are still a thing. You could name mangle the method with the @unsafe annotation and then remangle it inside the unsafe { } block. An alternate trick might be to abuse the package private somehow to do it, but not obvious to me how.

johnynek avatar Sep 24 '15 19:09 johnynek

The annotation is a great idea (regardless of what else we do).

I'm going to see if I can get Travis successfully building the branch @tixxit linked. Right now it doesn't look like PositiveInt is a value class but I bet we can fix that.

I think the longterm strategy for doing this might involve passing evidence that unsafe methods will be called safely, either via dependent types, type tagging, or something else. If we play our cards right with macro annotations we could signal which things could fail, and make the unsafe { ... } macro work a bit like the async { ... } macro (i.e. if the evidence was absent, it could synthesize the safety checks, and bail when any of them failed).

non avatar Sep 24 '15 19:09 non