kernel: add linalg functions #309
No reviewers
Labels
No Milestone
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: M-Labs/artiq-zynq#309
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "abdul124/artiq-zynq:add_linalg_functions"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds linalg functions from M-Labs/nac3#478
Do we really need to call the firmware for something as simple as transpose?
Should probably go to IRRT instead.
@ -435,6 +435,8 @@ static EXCEPTION_ID_LOOKUP: [(&str, u32); 12] = [
("IndexError", 9),
("UnwrapNoneError", 10),
("SubkernelError", 11),
("ValueError", 12),
I don't think this ValueError addition belongs in this PR?
@ -436,2 +436,4 @@
("UnwrapNoneError", 10),
("SubkernelError", 11),
("ValueError", 12),
("LinAlgError", 13),
Needs update in ARTIQ and PR there.
Will update ARTIQ first then and add these in an additional PR.
@ -345,2 +324,2 @@
api!(linalg_try_invert_to = linalg_try_invert_to),
api!(linalg_wilkinson_shift = linalg_wilkinson_shift),
api!(np_transpose = linalg::np_transpose),
api!(np_dot = linalg::np_dot),
As far as I can tell, these two are trivial and do not need firmware calls. Suggest moving to IRRT.
Re-implemented
np_transpose
andnp_reshape
using LLVM IR in core. Will movenp_dot
as well and remove these from firmware.Manual calls to LLVM builder or IRRT? Which way is best?
Since other functions like
np_array
andnp_zero
are already written using LLVM IR incore/codegen/numpy
I think keeping things consistent is more important.@ -0,0 +25,4 @@
}
}
/// # Safety
Remove all those "safety" headers, C calls are never "safe" as per Rust's definition.
Keep the descriptions of what the arguments should point to, however.
Adding safety headers over unsafe functions is required when using stable rust. Should I still remove those?
Hmm ok, keep them then.
@ -0,0 +321,4 @@
"last 2 dimensions of the array must be square: {0} != {1}",
dim1[0], dim1[1]
);
artiq_raise!("LinAlgError", err_msg);
Is it really OK to use a formatted string in artiq_raise?
Where is the memory allocated?
Not sure about the memory allocation, but formatted string is being used in other parts of code base (
@ -0,0 +310,4 @@
let out_z = out_z.as_mut().unwrap();
if mat1.ndims != 2 {
let err_msg = format!("expected 2D Vector Input, but received {}-D input", mat1.ndims);
2D vs. 2-D inconsistency.
Updated.
0664f3336b
to9c82781217
@ -470,1 +470,3 @@
($name:expr, $message:expr) => {{ artiq_raise!($name, $message, 0, 0, 0) }};
($name:expr, $message:expr) => {{
artiq_raise!($name, $message, 0, 0, 0)
}};
This change does not belong in this PR (and maybe should not be made at all).
Friendly reminder, just run
cargo fmt --all
after you make your changes. Manual formatting changes are unnecessary.@ -344,3 +325,2 @@
// linalg
api!(linalg_try_invert_to = linalg_try_invert_to),
api!(linalg_wilkinson_shift = linalg_wilkinson_shift),
api!(np_linalg_matmul = linalg::np_linalg_matmul),
Still needs removal.
9c82781217
toce8b8949bc
I just ran build latest master, and got these warnings:
rust-stable allows nested unsafe blocks for clarity, but with rust-nightly having nested unsafe is discouraged (hence the warning). Will create another PR to remove the nested unsafe.