NetLogo / Tortoise

Compiler and runtime engine for NetLogo models that runs in JavaScript 🐢
https://netlogoweb.org
Other
56 stars 27 forks source link

Add Matrix Extension #218

Closed iloveivyxuan closed 4 years ago

iloveivyxuan commented 4 years ago

This PR is the matrix extension.

It is based on the previous branch wip-matrix and Issue #197. Because of the huge difference between then and now, I couldn't work on the previous branch. So I created a new branch, but it would lose previous contributing history.

Following is the summary of what I've done in this PR.

Notes

I didn't implement eigenvectors calculations: real-eigenvalues, imaginary-eigenvalues, and eigenvectors. Though vectorious-6.0.0 adds API matrix.eig() to calculate real-eigenvalues and eigenvectors, it has run-time problems when results include decimals. It only works well when results are integers. I plan to check it again and create an issue, waiting for the developer's answer.

I used the beta version 6.0.0 of the library instead of the released 5.3.0. Because only the version 6.0.0 support calculating eigenvectors. The developer makes breaking changes at 6.0.0. Documentation has also changed to the new version. I think version 6.0.0 is ready and will be released in the near future.

Before installing vectorious, we need to install lapack on our own computer first, by running

$ brew install lapack

Because vectorious depends on nlapack to do solve, inv(inverse), and eig(eigenvectors), which needs lapack to be installed as a prerequisite.

Here is something I'm not sure about and may need to be improved.

First is the floating-point arithmetic problem, mentioned in #197. APIs that calculates matrix may have an issue where their floating-point arithmetic is not 100% accurate with desktop NetLogo, like solve, inverse, regress. It is because the library vectorious uses Float32Array as the default data structure, which only has 7 significant digits. But Desktop Nelogo uses 64-bit floating point values. I guess I can force using Float64Array to have exactly the same results as the Desktop, but I don't know whether it is necessary or really matter to do so.

I thought about rounding results in 3 decimal places or so to make results seem accurate. But it doesn't seem like a good way. Any suggestions here?

The second is about input validation. I realize there're many corner cases that I need to concern, whether the input is a List[List[Number]] or Matrix, or whether numeric values are within boundary. I implemented two functions, checkNestedList() and validate(). But I'm concerned I didn't cover all possible invalid situations. I was also wondering whether there's a better way to check input types.

Another thing is exception handling. Now I simply throw an error when detecting an invalid input. But there are other exceptions that would happen when calling APIs from vectorious. For example, matrix.inv() may cause an error if input matrix is not square. Do I need to catch these errors and customize them in order to align with Desktop or give users more useful information?

# Matrix => Matrix
inverse = (matrix) ->
  validate(matrix)
  try
      matrix.inv()
  catch e
      throw new error(Extension exception:  #{e})

Lastly, I paid attention to data leak problems. I wrote some tests and didn't find anything wrong. I rechecked the library and think the library avoids data from leaking. I'm not sure whether I considered it thoroughly.

iloveivyxuan commented 4 years ago

Okay, I added dummy implementation for eigenvalues and eigenvectors. I've pushed it.

For local dependency lapack

Yeah, you are right. I recheck the library and find lapack does is optional. All APIs in vectorious can be successfully used without nlapack and nblas. It should be safe to ignore these two optional dependencies.

I'm a little worried that the installer errors may scare other developers. But I didn't find the right way to use yarn install --ignore-optional as a default way to install packages.

It would be so nice and helpful if you will add a switch to skip optional dependencies later.

LaCuneta commented 4 years ago

Thank you, looks good to me!