-
Notifications
You must be signed in to change notification settings - Fork 47
Magicl NG #63
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
Magicl NG #63
Changes from 105 commits
b6cafac
46815b3
9ac5afe
89b3ada
55bb800
1a3f550
e8c30b7
d379d37
f6ea27e
35fcd3c
708ac46
5d5b183
d6faa05
4fbc577
6373563
c73430e
4399d78
f6a69fb
03365ca
b4c14d0
5558beb
83a5c47
60a3e4f
6f96e17
6b656aa
03986ad
ec93aa7
4eb5b03
77b8297
46e75ff
263d2b5
71f82ff
c259a7f
e3e7db5
2815c4c
5d023eb
9cdd3f5
47503eb
d7f9f6d
ee315a7
915df12
0158177
20969eb
8e8b423
ac08848
8505e1a
c0affa9
8854023
41825f1
2f31528
d98481e
0b1f445
64e43b1
c9d7590
39e55f7
b236a1a
00b4737
f13ccd3
c9099f5
d8e95e7
13ab91a
53b9b04
a60a9f5
4d40da0
3820678
7520aa2
453df93
cb99342
2c26e9c
79811ce
345d758
aaa000e
eb653cf
bb867a7
d3a480b
4df0b7c
39e68ed
f3efe6f
7c40812
8525dd8
4d119fb
631c658
6e28586
e5ba944
7b87ea8
c5d2904
ee2dcc7
a59c28f
cda9a1e
7721785
777eff0
e6420b3
52ac9b7
a11339c
524a087
82d87bd
d2303a3
400a40c
4ed0883
e0c0860
4b17a3b
c27a0fa
4b97964
fb1162b
a765019
923f016
2df9817
5fecbc2
339e7d2
415808a
015e273
cfbd144
9c4e1ac
9ab4a0d
837f859
4a1b206
7597577
65f6de6
33973d5
30e518f
6d04eb3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
"0.6.5" | ||
"0.7.0" | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
# High Level Bindings | ||
|
||
The purpose of the high-level magicl bindings is to allow for MATLAB-like multidimensional arrays in lisp. | ||
|
||
## Constructors | ||
|
||
The construction of tensors can be done with any of the given constructors. The constructors take a shape and arguments for method of construction. | ||
|
||
Tensors are specialized on both the shape and the element type. The class of a tensor will be of the form `$CLASS/$TYPE` (e.g. `MATRIX/DOUBLE-FLOAT`). | ||
|
||
| Number of dimensions | Tensor Class | | ||
|----------------------|-----------------| | ||
| 1 | `VECTOR` | | ||
| 2 | `MATRIX` | | ||
| * | `TENSOR` | | ||
|
||
| Element Type | Class Suffix | | ||
|--------------------------|------------------------| | ||
| `SINGLE-FLOAT` | `SINGLE-FLOAT` | | ||
| `DOUBLE-FLOAT` | `DOUBLE-FLOAT` | | ||
| `(COMPLEX SINGLE-FLOAT)` | `COMPLEX-SINGLE-FLOAT` | | ||
| `(COMPLEX DOUBLE-FLOAT)` | `COMPLEX-DOUBLE-FLOAT` | | ||
| `(SIGNED-BYTE 32)` | `INT32` | | ||
|
||
The type of the elements of the tensor can be specified with the `:type` keyword, or the constructor will attempt to find an appropriate type from the given arguments. The default element type for a tensor is `DOUBLE-FLOAT`. | ||
|
||
The layout of the tensor (column-major or row-major) can be specified with the `:layout` keyword. This affects the underlying storage of the tensor and will affect how it carries out operations with LAPACK. | ||
|
||
|
||
## Other Library Equivalents | ||
|
||
This table was adapted largely from the [NumPy Equivalents Table](https://docs.scipy.org/doc/numpy/user/numpy-for-matlab-users.html#linear-algebra-equivalents). | ||
|
||
### Basic Accessors | ||
|
||
| MAGICL | MATLAB | NumPy | Description | | ||
|----------------|------------|-------------------------|---------------------------------------------------------------| | ||
| `(order a)` | `ndims(a)` | `ndim(a)` or `a.ndim` | Get the number of dimensions of the array. | | ||
| `(size a)` | `numel(a)` | `size(a)` or `a.size` | Get the number of elements of the array. | | ||
| `(shape a)` | `size(a)` | `shape(a)` or `a.shape` | Get the shape of the array. | | ||
| `(tref a 1 4)` | `a(2,5)` | `a[1, 4]` | Get the element in the second row, fifth column of the array. | | ||
colescott marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Constructors | ||
|
||
| MAGICL | MATLAB | NumPy | Description | | ||
|-------------------------------------------------|--------------------|-----------------------------------|--------------------------------------------------------------------------------------| | ||
| `(from-list '(1d0 2d0 3d0 4d0 5d0 6d0) '(2 3))` | `[ 1 2 3; 4 5 6 ]` | `array([[1.,2.,3.], [4.,5.,6.]])` | Create a 2x3 matrix from given elements. | | ||
| `(zeros '(2 3 4))` or `(const 0d0 '(2 3 4))` | `zeros(2,3,4)` | `zeros((2,3,4))` | Create a 2x3x4 dimensional array of zeroes of double-float element type. | | ||
| `(ones '(3 4)` or `(const 1d0 '(3 4))` | `ones(3,4)` | `ones((3,4))` | Create a 3x4 dimensional array of ones of double-float element type. | | ||
| `(eye 1d0 '(3 3))` | `eye(3)` | `eye(3)` | Create a 3x3 identity array of double-float element type. | | ||
| `(from-diag a)` | `diag(a)` | `diag(a)` | Create a square matrix from the diagonal entries in `a` with zeroes everywhere else. | | ||
| `(rand '(3 4))` | `rand(3,4)` | `random.rand(3,4)` | Create a random 3x4 array. | | ||
|
||
### Basic Operations | ||
|
||
| MAGICL | MATLAB | NumPy | Description | | ||
|------------|----------|------------------|-----------------------------| | ||
| `(@ a b)` | `a * b` | `a @ b` | Matrix multiplication | | ||
| `(.+ a b)` | `a + b` | `a + b` | Element-wise add | | ||
| `(.- a b)` | `a - b` | `a - b` | Element-wise subtract | | ||
| `(.* a b)` | `a .* b` | `a * b` | Element-wise multiply | | ||
| `(./ a b)` | `a./b` | `a/b` | Element-wise divide | | ||
| `(.^ a b)` | `a.^b` | `np.power(a,b)` | Element-wise exponentiation | | ||
|
||
### Linear Algebra | ||
|
||
| MAGICL | MATLAB | NumPy | Description | | ||
|---------------------------------------------------------|-------------------|----------------------------------------------------------------|--------------------------------------------| | ||
| `(det a)` | `det(a)` | `linalg.det(a)` | Determinant of matrix | | ||
colescott marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| `(trace a)` | `trace(a)` | `trace(a)` | Trace (sum of diagonal elements) of matrix | | ||
colescott marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| `(upper-triangular a)` | `triu(a)` | `triu(a)` | Upper triangular part of matrix | | ||
braised-babbage marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| `(lower-triangular a)` | `tril(a)` | `tril(a)` | Lower triangular part of matrix | | ||
colescott marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| `(transpose a)` | `a.'` | `a.transpose()` or `a.T` | Transpose of matrix | | ||
| `(conjugate-transpose a)` or `(dagger a)` | `a'` | `a.conj().transpose()` or `a.conj().T` | Conjugate transpose of matrix | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Request: define As a matter of fact, the |
||
| `(inv a)` | `inv(a)` | `linalg.inv(a)` | Inverse of matrix | | ||
colescott marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| `(svd a)` (Returns `(VALUES U SIGMA Vt)`) | `[U,S,V]=svd(a)` | `U, S, Vh = linalg.svd(a), V = Vh.T` | Singular value decomposition of matrix | | ||
| `(eig a)` (Returns `(VALUES EIGENVALUES EIGENVECTORS)`) | `[V,D]=eig(a)` | `D,V = linalg.eig(a)` | Eigenvalues and eigenvectors of matrix | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to only track the real parts of the eigenvalues and eigenvalues. For example,
Compare with numpy:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems to work when the type is complex.
I think this brings up the question of if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Also, for the purposes of getting this PR over the finish line, I don't view adding any more top-level routines as a requirement, but it's worth keeping in mind for the future.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW there is a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sometimes |
||
| `(qr a)` (Returns `(VALUES Q R)`) | `[Q,R,P]=qr(a,0)` | `Q,R = scipy.linalg.qr(a)` | QR factorization of matrix | | ||
| `(ql a)` (Returns `(VALUES Q L)`) | | | QL factorization of matrix | | ||
| `(rq a)` (Returns `(VALUES R Q)`) | | `R,Q = scipy.linalg.rq(a)` | RQ factorization of matrix | | ||
| `(lq a)` (Returns `(VALUES L Q)`) | | | LQ factorization of matrix | | ||
| `(lu a)` (Returns `(VALUES LU IPIV)`) | `[L,U,P]=lu(a)` | `L,U = scipy.linalg.lu(a)` or `LU,P=scipy.linalg.lu_factor(a)` | LU decomposition of matrix | | ||
| `(csd a)` (Returns `(VALUES U SIGMA VT)`) | | | Cosine-sine decomposition of matrix | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a major version bump, btw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for pre-1.0 releases, major/minor doesn't have the same meaning, but generally bumping the minor is treated similarly to bumping the major in a post-1.0 release?
You could argue that magicl should already have been on a post-1.0 release, but that's a different story.
In practice I don't know how many downstream consumers magicl has besides quilc/qvm, so maybe it doesn't matter much, but I would lean towards keeping it pre-1.0.0 for at least a few more releases to give time for the new API to settle and shake out bugs before going to 1.0.
It just seems weird for something so new and not quite "stable" yet to get tagged as the 1.0 release. That is my $0.02 anyway, but I also would not ragequit if we made this 1.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't stay 0.x forever. Shaking out bugs in a big release is what we're supposed to be doing now? i.e. what if magicl was already 1.x and we wanted to make this release? You wouldn't then say we should make a minor 1.y release to shake out the bugs and only then do a 2.0? Or would you? Genuine question!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if it's what we supposed to be doing, but the reality is:
It'd be surprising if such massive changes didn't have a few bugs that need shaking out. See @kilimanjaro's comments above from just a quick run-through of the high-level docs.
There are quite a few comments in this PR suggesting API additions and improvements for follow-on PRs, so it doesn't quite seem like the API is that stable yet, which what 1.0 implies to my mind.
I guess my view is that a 1.0 release should be approached asymptotically rather than in a seismic shift.
If magicl already had a 1.0 release, then this would be a 2.0-worthy change for sure, but it hasn't so it feels odd to me to label this the 1.0 release based on the fact that it introduces major breaking changes, rather than basing it on the fact that the API feels in some sense "stable enough to be called 1.0", although the quotes there mean I don't have some clear criteria for what "stable enough" would mean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that "1.0.0" is overloaded with symbolism. Does semantic versioning just not apply for 0.y.z?
Not that it matters. I don't mind if we go to 0.7.0 rather than 1.0.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a semver expert, I just play one on TV, but according to their faq pre-1.0 releases are treated differently
https://semver.org/#how-should-i-deal-with-revisions-in-the-0yz-initial-development-phase
Of course, the very next bullet point makes the argument that magicl should probably have already been on a post-1.0 release...
I think one thing we both agree on is that it probably doesn't matter much if quilc and qvm are the only downstream consumers :)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will be bumping to 1.0.0edit:
appleby convinced me. It will stick with 0.7.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm more or less with appleby on this one as well. Version 1.0.0 feels special because it is special. My only objection to this being 1.0.0 is that we don't have much working experience with the API, outside of this code review, and so it is hard to anticipate what follow-up work will be on the "wish list".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I concur.