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

Matrix Library Pull Request #1599

Open
wants to merge 43 commits into
base: horizon
Choose a base branch
from

Conversation

AnirudhNarsipur
Copy link

Hi,

As was requested by the core Pyret team, William Sun and I wrote this library containing core linear algebra features.

What The Library Does :

  • Core Linear Algebra Functions (Inverse , Determinant , Orthogonalization)
  • Manipulate matrices and vectors
  • An advantage of Pyret's number system is that Inverse(A) * A = Identity.

What The Library Does Not Do :

  • Handle Complex Numbers, Tensors
  • More Complex Linear Algebra / any sort of statistical functions (Eigenvalues ... )

Setup

  • There are two datatypes Matrix and Vector . A Matrix is a nxn 2D array of numbers while a vector is equivalent to an array of numbers
  • Representation wise both matrices and vectors are a 1D Javascript Array. These are then appropriately wrapped to create Pyret objects
  • You create a matrix by : [mat(2,2):12,4,5,6] and a vector by [vector(3):1,3,-42]
  • Generally all operations are functions. However basic arithmetic (+,-,*) are available as methods along with element access. These are also available as functions.

Naming Conventions

  • Most functions are of the formfunc-matsuch as rref-mat . However some functions are given by just their name.

Phillip Blair's Library
A few years ago Phillip Blair wrote a matrix library (https://github.com/brownplt/pyret-lang/blob/8bf1d79c283d50a04daae1d3e4a7682e606c1c02/src/arr/trove/matrices.arr). Ours is equivalent , the critical difference being that it is Javascript rather than Pyret. You can find the modern version of Phillip's library compliant with 2021 (https://code.pyret.org/editor#share=1_UVB6S9ad3F9KRcrdNx3yrUyAw-rGdfo&v=1599623)

This makes a huge performance difference. The Pyret version times out for matrices as small as 50x50 (takes > 5mins) whereas the Javascript version (ours) does not show any lag.

Issues To Address Before Adding

  • Mapping functions such as build-mat use safeCalls to avoid blowing up the stack. However Pyret decides at some time to just halt the computation. So :
  • build-mat(30,30,lam(i,j): (3 + i) / (5 + j ) end) (Building a 30x30 matrix) works fine
  • build-mat(40,40,lam(i,j): (3 + i) / (5 + j ) end) (Building a 40x40 matrix) returns [mat(40,40): ]

I'm unsure as to how to solve this while still keeping these functions safe.

Other Things To Address

  • Right now matrix output is displayed as a 1d List

Screen Shot 2021-06-12 at 11 47 22 PM

  • Matrices are keeping with Pyret principles immutable. Is a mutable version required or should matrices be by default mutable .

Anirudh Narsipur

Copy link
Member

@blerner blerner left a comment

Choose a reason for hiding this comment

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

This is not ready yet. There are lots of missing or wrong annotations to be checked, incorrect logic in several places, some dubious stack-safety issues to be addressed, and more tests are definitely needed (especially since some functions haven't been tested at all). My comments below are a starting point for you; there are likely more issues to be addressed. When you're dealing with annotations, stack safety, equality, etc, you have to cultivate a certain kind of "skeptical paranoia", as in, "What could a clueless or malicious user do, that would violate some assumption I made and cause my code to break? What could an unfortunate scheduling in the browser do to force my code to pause at an inconvenient point?" I'm happy to set up a meeting time to go over the code with you and review what needs fixing, if you'd like, but I do want you to go through the feedback below and try to categorize it into big clumps of related comments (e.g. "annotations missing or wrong", "tests missing or incomplete", "stack safety wrong", etc), and then go back through your code function-by-function and see which categories apply to each function.

Is there documentation to go with any of this code yet, or not yet?

src/js/trove/matrix.js Outdated Show resolved Hide resolved
src/js/trove/matrix.js Show resolved Hide resolved
src/js/trove/matrix.js Outdated Show resolved Hide resolved
src/js/trove/matrix.js Show resolved Hide resolved
return runtime.safeCall(
function(){return recEq.app(self.$underlyingMat[i],other.$underlyingMat[i])},
function(result){
if (runtime.ffi.isNotEqual(result)) {
Copy link
Member

Choose a reason for hiding this comment

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

This logic is wrong. Depending on the order you have values in the matrix, you could have a roughnum followed by an exactnum. Consider [mat(2, 2): 1, 2, ~3, 4] and [mat(2, 2): 1, 2, ~5, 4]. The roughnums will compare as Unknown, which you ignore, and then the recursive call that compares 4 will say they're Equal, and you'll return that as the answer. Alternately, if the last values happened to not be equal, then you'd return NotEqual which would happen to be correct here. And finally, if the values happened to have the roughnums last, then you'd return Unknown which would happen to be correct. You need to use the logic of equal-and to combine your answers here. And you likely want to use raw_array_fold instead of rolling your own loop.

Copy link
Author

Choose a reason for hiding this comment

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

Resolved. I'm not using raw_array_fold because we want to terminate early if not equal.

src/js/trove/matrix.js Outdated Show resolved Hide resolved
src/js/trove/matrix.js Outdated Show resolved Hide resolved
src/js/trove/matrix.js Outdated Show resolved Hide resolved
src/js/trove/matrix.js Show resolved Hide resolved
src/js/trove/matrix.js Outdated Show resolved Hide resolved
@AnirudhNarsipur
Copy link
Author

I've addressed all comments and increased test coverage. There are still 2 issues :

  • build-mat(100,100,lam(i,j): i + j end) times out. I think I've wrapped it in safeCalls appropriately as suggested.
  • I chunked up multiplication which works for the most part. However it's creating some strange behaviour in exp-mat which raises a matrix to a power by repeatedly multiplying it ( line 873):
res = duplicateMatrix(self);
            for (i = 1; i < n; i++) {
                res = multMatrix(res, self);
            }

Calling exp-mat(a,3) for a = [mat(2,2):1,~4,-5,6.53] results in a roughnum overflow error. For some reason the loop instead of running just twice runs dozens of times and causes this error.

src/js/trove/matrix.js Outdated Show resolved Hide resolved
src/js/trove/matrix.js Outdated Show resolved Hide resolved
@AnirudhNarsipur
Copy link
Author

Addressed both comments and made similar fix in map-mat. I've left the other multMatrix calls unchanged for now but my understanding is that each of them need to individually wrapped but where does this end ? If I have f calling multMatrix and g calling f . Do both g and f now need to be wrapped in safeCalls?

jpolitz added a commit that referenced this pull request Jul 15, 2021
@schanzer schanzer mentioned this pull request May 10, 2024
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.

None yet

3 participants