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

Specialize Grid for Int and Long #3428

Merged
merged 2 commits into from Oct 19, 2021

Conversation

pomadchin
Copy link
Member

@pomadchin pomadchin commented Oct 19, 2021

Overview

scala> val tile = ShortUserDefinedNoDataArrayTile(Array(), 10, 10, ShortUserDefinedNoDataCellType(-1))
tile: geotrellis.raster.ShortUserDefinedNoDataArrayTile = ArrayTile(10,10,int16ud-1)

scala> trait C { def cols: Int = (tile: CellGrid[Int]).cols }
defined trait C

scala> :javap C
  // ...
  public default int cols();
    descriptor: ()I
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: getstatic     #71                 // Field $line3/$read$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$.MODULE$:L$line3/$read$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$;
         3: invokevirtual #75                 // Method $line3/$read$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$.tile:()Lgeotrellis/raster/ShortUserDefinedNoDataArrayTile;
         6: invokevirtual #80                 // Method geotrellis/raster/CellGrid.cols$mcI$sp:()I
         9: ireturn
      LineNumberTable:
        line 27: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0      10     0  this   L$line4/$read$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$C;
  // ...

Checklist

  • ./CHANGELOG.md updated, if necessary. Link to the issue if closed, otherwise the PR.

Closes #3427

@@ -19,7 +19,7 @@ package geotrellis.raster
import spire.math.Integral
import spire.implicits._

abstract class Grid[N: Integral] extends Serializable {
abstract class Grid[@specialized(Int, Long) N: Integral] extends Serializable {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warn] geotrellis/raster/src/main/scala/geotrellis/raster/GridExtent.scala:36:7: class Grid must be a trait. Specialized version of class GridExtent will inherit generic geotrellis.raster.Grid[Int]
[warn] class GridExtent[@specialized(Int, Long) N: Integral](
[warn]       ^
[warn] one warning found

Copy link
Member Author

@pomadchin pomadchin Oct 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scala/bug#8405 Grid should be converted into a trait.

@pomadchin pomadchin changed the title [WIP] Specialize Grid on Int and Long Specialize Grid on Int and Long Oct 19, 2021
@@ -33,13 +33,13 @@ import spire.implicits._
* The constructor will throw [[java.lang.IllegalArgumentException]] if the provided extent and cell size do not match the
* provided cols and rows (dimensions).
*/
class GridExtent[@specialized(Int, Long) N: Integral](
class GridExtent[N: Integral](
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a specialized trait is enough!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting... That's really good to know.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

val extent: Extent,
val cellwidth: Double,
val cellheight: Double,
val cols: N,
val rows: N
) extends Grid[N] with Serializable {
) extends GridIntegral[N] {
Copy link
Member Author

@pomadchin pomadchin Oct 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trait Grid[N] extends Serializable

@pomadchin pomadchin changed the title Specialize Grid on Int and Long Specialize Grid for Int and Long Oct 19, 2021
Copy link
Contributor

@echeipesh echeipesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice bit of detective work.

Copy link
Member

@metasim metasim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for doing all this work!

@pomadchin pomadchin merged commit ff33a6a into locationtech:master Oct 19, 2021
@pomadchin pomadchin deleted the fix/grid-boxing branch October 19, 2021 14:09
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.

ShortUserDefinedNoDataArrayTile.cols()/rows() calls scala/runtime/BoxesRunTime.boxToInteger
3 participants