dragonfly-ai / slash

Linear Algebra and Statistics library for Scala.js, JVM, and Native.
https://dragonfly-ai.github.io/slash/
Apache License 2.0
25 stars 4 forks source link

Some questions : #5

Closed Quafadas closed 1 year ago

Quafadas commented 1 year ago

This might be better suited to a discussion than an issue, but I don't think discussions are open on this repo?

Some observations:

Getting started took a little more time than was necessary, as I had to go to maven to figure out the version and org //> using lib ai.dragonfly::vector::0.101

Perhaps could be added to the top of the readme?

Vec[3](true, false, true) is a compile error. Can I assume, that this library is specialised to double vectors as a deliberate design choice?

val b = NArray(false, true) would be it's equivalent?

Currently, I can see how to index into a single entry, x(5). Is it possible to index into multiple entries, or would that be a custom for loop?

Spiritually,

val v = (1 to 10).map(_.toDouble).toArray
 val x = Vec[10]( v )
 val s = Vec(5,6)
 val slicedVec = x(s) // compile error... ideally would be a Vec[2](5, 6)

A Naive implementation fails, because Vec doesn't implement map

println(for (idx <- s) yield{ x(idx) }) // compile error

Is there a canonical approach to such a thing? I can see that NArrays get ArrayOps methods ... is it the intent that Vectors would, as well?

  val x = Vec.fill[10](2.0)
  val y = Vec.fill[10](1.0)
  val z = x.add(y)

  println("---- x ")
  println(x.show)
  println("---- y ")
  println(y.show)
  println("---- z ")
  println(z.show)

Prints

---- x 
《¹⁰↗〉3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0〉
---- y 
《¹⁰↗〉1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0〉
---- z 
《¹⁰↗〉3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0〉

Is that expected? I assume so? It would appear, that the "add" operation has mutated the value of x. In contrast, x + y, does not.

Is it a "convention" that a .method, may mutate it's inputs? In Julia, a method which operates on its inputs would be called add!, to make this very clear.

Further, my personal preference (which you may feel free to ignore), is that a function which mutates its inputs should return Unit, or have an = in it's name. i.e. be purely side effecting. This is largely because of scalas immutable by default behaviour - which I feel a little this quietly subverts, and thus can surprise the unwary.

This is a personal preference, but in my opinion, the combination of these two changes could make it much clearer that "Here Be Dragons", with the mutable methods.

Quafadas commented 1 year ago

P.S. I finally had time to have a look ! What's there so far looks pretty cool :-). Feedback is intended to be constructive - there's no "obligation" to do anything or even read it if you don't want to!

dragonfly-ai commented 1 year ago

Getting started took a little more time than was necessary, as I had to go to maven to figure out the version and org //> using lib ai.dragonfly::vector::0.101

I brought this point up in the scala Discord, and @SethTisue suggested doing it this way because the current release is written on the right side of the github page. I can try to put current version numbers in there, though, I just forget almost every time. I'll create a release checklist.

Quafadas commented 1 year ago

Getting started took a little more time than was necessary, as I had to go to maven to figure out the version and org //> using lib ai.dragonfly::vector::0.101

I brought this point up in the scala Discord, and @SethTisue suggested doing it this way because the current release is written on the right side of the github page. I can try to put current version numbers in there, though, I just forget almost every time. I'll create a release checklist.

Hmmm... I wouldn't do that. sbt-typelevel must have something. I know it publishes a website, and I think it can automagically include the version number in it. I'll look into it... but let's keep the release process easy. I thin kit's also possible to add a badge to the readme with the info somehow. I'll look into something and PR the version number ?

https://github.com/dragonfly-ai/vector/pull/6

dragonfly-ai commented 1 year ago

Vec[3](true, false, true) is a compile error. Can I assume, that this library is specialised to double vectors as a deliberate design choice?

Yes! Basically because it is a cross-project library and JavaScript kind of only supports Double precision floating point numbers.

I originally tried to make Vector support all numeric types, but a lengthy discussion on the Scala Discord with @s5bug and @BalmungSan convinced me not to bother.

Do you have a use case for other numeric types or Boolean? If so, how would you implement it without losing the performance Vector currently benefits from?

dragonfly-ai commented 1 year ago

sbt-typelevel must have something

image

Quafadas commented 1 year ago

Vec[3](true, false, true) is a compile error. Can I assume, that this library is specialised to double vectors as a deliberate design choice?

Yes! Basically because it is a cross-project library and JavaScript kind of only supports Double precision floating point numbers.

I originally tried to make Vector support all numeric types, but a lengthy discussion on the Scala Discord with @s5bug and @BalmungSan convinced me not to bother.

Do you have a use case for other numeric types or Boolean? If so, how would you implement it without losing the performance Vector currently benefits from?

That's all fine for me - just checking the assumption :-).

dragonfly-ai commented 1 year ago

Is that expected? I assume so? It would appear, that the "add" operation has mutated the value of x. In contrast, x + y, does not.

Exactly!

inline def + (v0: Vec[N]): Vec[N] = copy.add(v0)

inline def += (v0: Vec[N]): Vec[N] = add(v0)

def add(v0: Vec[N]): Vec[N] = {
  var i = 0
  while (i < dimension) {
    thisVector(i) = thisVector(i) + v0(i)
    i = i + 1
  }
  thisVector
}

Same for -, *, and /.

I guess I should explain that explicitly in the documentation.

dragonfly-ai commented 1 year ago

This is a personal preference, but in my opinion, the combination of these two changes could make it much clearer that "Here Be Dragons", with the mutable methods.

I like being able to chain operations: v.add(v1).times(0.1).normalize() but maybe we could change add to addEquals or addInPlace.

dragonfly-ai commented 1 year ago

val v = (1 to 10).map(_.toDouble).toArray

For faster and more concise initialization, how about:

val v = (1 to 10).map(_.toDouble).toArray.asInstanceOf[Vec[10]]

or:

val v = Vec.tabulate[10](_.toDouble)
dragonfly-ai commented 1 year ago

val slicedVec = x(s) // compile error... ideally would be a Vec[2](5, 6)

I haven't come across a use case for slicing math vectors. I know it's common in collections, but is it useful to think of linear algebra vectors as collections?

dragonfly-ai commented 1 year ago

Vec doesn't implement map

Yeah, it should support map. I have my doubts about filter, though.

Quafadas commented 1 year ago

This is a personal preference, but in my opinion, the combination of these two changes could make it much clearer that "Here Be Dragons", with the mutable methods.

I like being able to chain operations: v.add(v1).times(0.1).normalize() but maybe we could change add to addEquals or addInPlace.

This definitely makes sense - I like Julias add!, but YMMV :-).

Quafadas commented 1 year ago

val slicedVec = x(s) // compile error... ideally would be a Vec[2](5, 6)

I haven't come across a use case for slicing math vectors. I know it's common in collections, but is it useful to think of linear algebra vectors as collections?

I think it can be - although I would probably agree it departs from linear algebra. I'm not sure how to implement something like TVaR (sensibly!) without being able to index and slice, though?

i.e. get every number in excess (or below) a given threshold and find the average...

There are other use cases I haven't thought through so much, something like rank correlation?