Long 4D arrays in Convolutional constructor: strides, dilations, kernelSize. What should we do with that?

[zaleslaw]

Firstly, it was made in many cases like just wrappers for underlying TF Java API, which is full of these long arrays, as you see.
But it looks like nobody wants to use these 4d arrays in real life.

Let's try to answer here, in this thread on a few questions:

1. Should it be reduced to 2 or 1 numbers instead of 4?
2. What is the type for this number should be? Int or Long or Byte?
3. What is the default value for these parameters that should be set up?
4. What layers have the same issue and should be refactored?
5. How will it be converted under the hood to low-level TF calls?
6. Should we leave users the ability to pass all 4 parameters or 2 parameters in the Conv2D case?
7. Where in the KotlinDL API Long should be used and where Int should be used? Let's fix the rule for that in this thread.

Hi @avan1235 @mkaze you both asked me about that, it's time to discuss

[avan1235]

I consider only conv2d case but the other are similiar.

1. I don't see the actual use of first and last elements of strides and dilations (so imo they should be just changed to have 2 elements in order not to make the users confused). The kernel size now have valid dimension (2 elements array) but there should be possibility to create it with single value. We can try to achieve all of this with extra DSL and types for size e.g.

public data class Size1D(val v: Long)

public data class Size2D(val w: Long, val h: Long) {
    public constructor(b: Long) : this(b, b)
}

public fun Int.both(): Size2D = Size2D(this.toLong())

public val Int.D1: Size1D get() = Size1D(this.toLong())

public val Int.D2: Size2D get() = Size2D(this.toLong())

public infix fun Int.x(o: Int): Size2D = Size2D(this.toLong(), o.toLong())

public infix fun Size1D.x(o: Size1D): Size2D = Size2D(this.v, o.v)

and then use it as follows

val someKernel1D = 3.D1

val smallKernel2D = 1 x 1 // this is my love from the first sight :D
val mediumKernel2D = 2.both()
val largeKernel2D = 3.D2
val fromKernel1D2D = someKernel1D x someKernel1D

2. If Tensorflow Java API uses long arrays we should imo keep everything in the same format internally (as long values) but for now we can create only the API to create the instances of Sizes from Int as it is simpler for user to create the instance of Size then (as at the presented example). In future if there would be need from the community to have some extra DSL to work also with long numbers there should be not a lot of work to introduce them (as internally we would work with long values as described) but for now I see no sense in having the API that is based on Long values
3. Imo we should learn from the leading libraries in which the default value is 1 for strides, dilations and the kernel size has to be given explicitly. Imo we should also require specifying the kernel size in every conv to make models code more readable
4. Consider all convolutional and pooling - all of them need analogous solution.
5. I would propose adding the extra (maybe internal if possible) method for converting Size to long[]
6. Imo they are not used so there is no need fr them to exists (but I might have missed something)
7. Do you actually need to have Long anywhere in API. I see no use case which would force me to use the long values for all these configurations so maybe Int should be visible in API while Long as internal backing field (as the actual role of KotlinDL is from my perspective simplify the everyday process of models management in Kotlin while considering the need of long values for all these 'configurations' for layers seems to be really expert and unusual situation)

[rnett]

I don't see any situation where you couldn't reduce it to two, you just have to expand it according to the channel format. You could maybe have a situation where some wants all 4 values for dilation but I've never seen anything like it.

When I've had to do this in the past, I've done something like (w/ better names):

sealed interface Expandable4{
    fun all(): Data4
}
data class Data4(val a: Long, val b: Long, val c: Long, val d: Long): Expandable4{
    override fun all() = this
}
data class Data2(val a: Long, val b: Long): Expandable4{
    override fun all() = Data4(a, b, a, b)
}
data class Data1(val a: Long): Expandable4{
    override fun all() = Data4(a, a, a, a)
}

fun Conv2d(kernelSize: Expandable4 = Data1(3), ...)

It makes more sense in some cases to turn Data4 into 2 Data2s.

It's more wrapping than I would like, especially for single int literals (which is usually what I see most), but I don't see a way around that besides concise object literals or collection literals.

[avan1235]

This wrapping is done once on the side of the library contributors (not users) so imo it should be done once but in a good way to simplify many future uses of library - so we should not worry about that aspect. I just would point that from my perspective it should be like

data class Data2(val a: Long, val b: Long): Expandable4{
    override fun all() = Data4(1, a, b, 1)
}

if I understand current code correctly.

[rnett]

Yeah, you're right for kernel size, but padding is [a, b, a, b] usually (i.e. pad height and width w/ the same values) and I think there's a few others. Well, [[a, b], [a, b]]. The nice part of using sealed interfaces for this is you could add a Expandable2x2 interface w/ a different expansion method and reuse the data classes, and so on for any different expansions. I'm honestly not sure whether it's worth it to have an Expandable4, since it seems like for kernel size and whatnot you could just use Expandable2 and figure out where to put the ones based on the channel format.

[mkaze]

Thanks for creating this discussion thread for this issue. Well, as I have mentioned earlier, personally I prefer:

• No convolution/pooling/strides/dilation over batch or channels dimensions (e.g. for Conv2D the kernel size should only be specified by its height and width) as they are very rarely used in building practical ML models these days (actually, I am not aware of an example). Also, this is a practice that both TF and PyTorch follow as well (at least in their high-level APIs).
• Using Int instead of Long for the public high-level API (and convert internally if necessary). If Int is sufficient and intuitive for these params, then what's the benefit of using Long? (Also, I am really curious to know why Long has been used in the low-level Java API).
• Using Int and IntArray (or List<Int>) as the type of these parameters, instead of creating more (fancy) abstractions which might potentially confuse the users (note: I am not sure which one of IntArray or List<Int> is better to use; one of the members of Kotlin team has suggested that using List is better than using Array, here: "As to the usage, good practice is to prefer using lists over arrays everywhere except for performance critical parts of your code").
• The following default values:
  • kernelSize: No default value; should be explicitly set by user.
  • poolSize: 2, because that's the most commonly used value.
  • strides:
    • for convolution layers: 1, because that's the most commonly used value.
    • for pooling layers: null, because in pooling commonly the stride is equal to the pool size and therefore when it's set to null we would default to the pool size value for stride and therefore the user won't need to set this explicitly (both TF and PyTorch follow this approach).
  • dilation: 1, because that's the most commonly used value.

A word of caution: all of the above points is coming from a person who is mostly new to Kotlin/Java (but knows a relatively fair amount about ML and TF/Keras). So I may not be the best person to comment on the API design issues. :)

[rnett]

I agree with everything here, except I'm not sure poolSize should have a default and I don't like using List<Int>. Using List<Int> over a data class trades some verbosity for runtime exceptions, which is almost never a good trade, and there's no good way to encode the expansion behavior besides docs. I do think whatever abstraction we use should be kept pretty simple, i.e. normal data classes, nothing too complex.

[zaleslaw]

@rnett @mkaze @avan1235 thanks for your active participation in the discussion. I suppose no new opinion will be added here.

Looks like we need to start from small steps like

1. long -> int
2. 4d arrays -> Pair or 2d array

I'd prefer an array of primitives instead of Lists here, due to the generic nature of lists instead of strong type in IntArray, for example.

As the second step, we could play in the experimental branch with the small hierarchy of data classes as @avan1235 proposed and ask some potential users about it.

I'm going to make an issue about that and make this refactoring (regarding the first step).