add nalgebra::linalg methods #478

Merged
sb10q merged 1 commits from support_nalgebra into master 2024-08-17 17:37:21 +08:00
Collaborator

Adds implementation of:

  1. try_invert_to https://docs.rs/nalgebra/latest/nalgebra/linalg/fn.try_invert_to.html
  2. wilkinson_shift https://docs.rs/nalgebra/latest/nalgebra/linalg/fn.wilkinson_shift.html

from nalgebra::linalg. nac3core/codegen/externfns provides implementation in rust (similar to irrt.cpp) for functions that will be made available at runtime. These are linked to the main program in nac3standalone/demo/run_demo.sh to allow the nalgebra::linalg functions to be tested.

Adds implementation of: 1. `try_invert_to` https://docs.rs/nalgebra/latest/nalgebra/linalg/fn.try_invert_to.html 2. `wilkinson_shift` https://docs.rs/nalgebra/latest/nalgebra/linalg/fn.wilkinson_shift.html from `nalgebra::linalg`. `nac3core/codegen/externfns` provides implementation in rust (similar to `irrt.cpp`) for functions that will be made available at runtime. These are linked to the main program in `nac3standalone/demo/run_demo.sh` to allow the `nalgebra::linalg` functions to be tested.
Owner

If the purpose of the DSO is to make the lli tests work, I would just get rid of the latter. The lli tests increase complexity and AFAICT don't really help find bugs.

If the purpose of the DSO is to make the lli tests work, I would just get rid of the latter. The lli tests increase complexity and AFAICT don't really help find bugs.
Owner

And the crate should not be named "externfns" but "nalgebra_c" (or similar) and should be part of demo, not core/codegen.

And the crate should not be named "externfns" but "nalgebra_c" (or similar) and should be part of demo, not core/codegen.
sb10q reviewed 2024-07-22 17:05:06 +08:00
Cargo.toml Outdated
@ -4,6 +4,7 @@ members = [
"nac3ast",
"nac3parser",
"nac3core",
"nac3core/src/codegen/externfns",
Owner

Alarm bells should be ringing when you compare this entry to the others.

Alarm bells should be ringing when you compare this entry to the others.
Owner

Again, move this out of nac3core. This is only used by nac3standalone/demo and this is where it should go.
I also doubt that clang xxy.so works to link an executable against a DSO. Did you test it? If it doesn't work just make it a static lib.

Again, move this out of nac3core. This is only used by nac3standalone/demo and this is where it should go. I also doubt that ``clang xxy.so`` works to link an executable against a DSO. Did you test it? If it doesn't work just make it a static lib.
Author
Collaborator

Again, move this out of nac3core. This is only used by nac3standalone/demo and this is where it should go.

Will move it.

I also doubt that clang xxy.so works to link an executable against a DSO. Did you test it? If it doesn't work just make it a static lib.

Yes, I tested it and clang does link against DSO. The DSO can also be linked with lli while Static lib will only work with clang, so I had compiled it to DSO instead.

> Again, move this out of nac3core. This is only used by nac3standalone/demo and this is where it should go. Will move it. > I also doubt that ``clang xxy.so`` works to link an executable against a DSO. Did you test it? If it doesn't work just make it a static lib. Yes, I tested it and clang does link against DSO. The DSO can also be linked with lli while Static lib will only work with clang, so I had compiled it to DSO instead.
Owner

Just call it linalg or similar, this name is too long.

Just call it linalg or similar, this name is too long.
Owner

And can it be moved to demo? Does it need to be at the top level cargo workspace?

And can it be moved to demo? Does it need to be at the top level cargo workspace?
Author
Collaborator

And can it be moved to demo?

Yes, will move it

Does it need to be at the top level cargo workspace?

If its removed from top level cargo workspace and introduced in some other workspace (nac3standalone here) cargo will not let it build as it considers this as double having two roots to the project. An alternative could be to have the linalg crate as another independent workspace which could be build separately.

> And can it be moved to demo? Yes, will move it > Does it need to be at the top level cargo workspace? If its removed from top level cargo workspace and introduced in some other workspace (`nac3standalone` here) cargo will not let it build as it considers this as double having two roots to the project. An alternative could be to have the `linalg` crate as another independent workspace which could be build separately.
Owner

I don't think it needs a workspace, it can be just a crate.

I don't think it needs a workspace, it can be just a crate.
sb10q reviewed 2024-07-22 17:06:55 +08:00
@ -1874,6 +1876,37 @@ impl<'a> BuiltinBuilder<'a> {
}
}
/// Build the functions `try_invert_to` and `wilkinson_shift`
Owner

Poorly written comment. If nalgebra methods are added or removed, it will go out of sync.

Poorly written comment. If nalgebra methods are added or removed, it will go out of sync.
sb10q reviewed 2024-07-22 17:08:02 +08:00
@ -263,6 +265,8 @@ impl PrimDef {
PrimDef::FunNpLdExp => fun("np_ldexp", None),
PrimDef::FunNpHypot => fun("np_hypot", None),
PrimDef::FunNpNextAfter => fun("np_nextafter", None),
PrimDef::FunTryInvertTo => fun("try_invert_to", None),
Owner

Insert blank line to separate from the other numpy stuff.

Insert blank line to separate from the other numpy stuff.
Owner

And please keep the naming scheme consistent. If this is replacing np.linalg.inv, it should be called np_linalg_inv, not try_invert_to.

And please keep the naming scheme consistent. If this is replacing ``np.linalg.inv``, it should be called ``np_linalg_inv``, not ``try_invert_to``.
sb10q reviewed 2024-07-22 17:08:33 +08:00
@ -265,2 +267,4 @@
PrimDef::FunNpNextAfter => fun("np_nextafter", None),
PrimDef::FunTryInvertTo => fun("try_invert_to", None),
PrimDef::FunWilkinsonShift => fun("wilkinson_shift", None),
PrimDef::FunSome => fun("Some", None),
Owner

(Not a problem coming from this PR)
Shouldn't Some be near Option? @derppening

(Not a problem coming from this PR) Shouldn't ``Some`` be near ``Option``? @derppening
Collaborator

Yeah it should. I think it's safe to move that over (as long as it doesn't regress any existing tests).

Yeah it should. I think it's safe to move that over (as long as it doesn't regress any existing tests).
sb10q reviewed 2024-07-22 17:10:17 +08:00
@ -144,0 +145,4 @@
try:
y = np.linalg.inv(x)
x[:] = y
except np.linalg.LinAlgError:
Owner

Why can't we just follow the numpy API and raise this exception on failure, instead of returning True/False?

Why can't we just follow the numpy API and raise this exception on failure, instead of returning True/False?
Owner

It is possible to raise a Python exception from ksupport.rs.

It is possible to raise a Python exception from ksupport.rs.
Author
Collaborator

Why can't we just follow the numpy API and raise this exception on failure, instead of returning True/False?

The implementation of try_invert_to in nalgebra::linalg returns True/False depending on whether the operation succeeded or not. I had the function return True/False to match the implementation in nalgebra::linalg.

> Why can't we just follow the numpy API and raise this exception on failure, instead of returning True/False? The implementation of `try_invert_to` in `nalgebra::linalg` returns True/False depending on whether the operation succeeded or not. I had the function return True/False to match the implementation in `nalgebra::linalg`.
Owner

The reference is numpy.

The reference is numpy.
sb10q reviewed 2024-07-22 17:11:58 +08:00
@ -144,0 +149,4 @@
return False
return True
def wilkinson_shift(x):
Owner

No corresponding numpy/scipy library function?

No corresponding numpy/scipy library function?
Owner

And generally I would try to follow numpy.linalg and the functions it provides, not implement random stuff from Rust nalgebra.

And generally I would try to follow ``numpy.linalg`` and the functions it provides, not implement random stuff from Rust nalgebra.
sb10q reviewed 2024-07-22 17:14:12 +08:00
@ -55,3 +58,3 @@
clang -c -std=gnu11 -Wall -Wextra -O3 -o demo.o demo.c
clang -lm -o demo module.o demo.o
clang -lm -o demo module.o demo.o $externfns
Owner

Can you really pass a DSO to clang and get it linked in?

Can you really pass a DSO to clang and get it linked in?
Owner

Anyway I would just make it a static lib.

Anyway I would just make it a static lib.
Author
Collaborator

If the purpose of the DSO is to make the lli tests work, I would just get rid of the latter. The lli tests increase complexity and AFAICT don't really help find bugs.

The DSO provides the implementation of nalgebra::linalg functions at runtime. Without it, the test_try_invert_to and test_wilkinson_shift test cases in standalone will not work.

> If the purpose of the DSO is to make the lli tests work, I would just get rid of the latter. The lli tests increase complexity and AFAICT don't really help find bugs. The DSO provides the implementation of `nalgebra::linalg` functions at runtime. Without it, the `test_try_invert_to` and `test_wilkinson_shift` test cases in standalone will not work.
abdul124 changed title from add nalgebra::linalg methods to WIP: add nalgebra::linalg methods 2024-07-22 17:44:32 +08:00
abdul124 force-pushed support_nalgebra from f30683578d to ea5f4fc698 2024-07-23 00:30:27 +08:00 Compare
sb10q reviewed 2024-07-23 08:59:58 +08:00
@ -0,0 +56,4 @@
let matrix2 = DMatrix::from_row_slice(dim2_0, dim2_1, data_slice2);
let mut result = DMatrix::<f64>::zeros(dim3_0, dim3_1);
matrix1.mul_to(&matrix2, &mut result);
Owner

Isn't that already implemented via the @ operator?

Isn't that already implemented via the ``@`` operator?
Author
Collaborator

Isn't that already implemented via the @ operator?

Yes it's already implemented. I added this to test the macro calls. Will remove it from the final version.

> Isn't that already implemented via the ``@`` operator? Yes it's already implemented. I added this to test the macro calls. Will remove it from the final version.
sb10q reviewed 2024-07-23 09:00:33 +08:00
@ -1838,0 +1879,4 @@
"",
)
.unwrap(),
"0:ValueError",
Owner

As I said before, you can raise exceptions from the Rust firmware.
It is better to write this in Rust than in LLVM IR calls.

As I said before, you can raise exceptions from the Rust firmware. It is better to write this in Rust than in LLVM IR calls.
Author
Collaborator

It is better to write this in Rust than in LLVM IR calls.

Ok.

> It is better to write this in Rust than in LLVM IR calls. Ok.
sb10q reviewed 2024-07-23 09:01:17 +08:00
@ -1838,0 +1865,4 @@
// The following constraints must be satisfied:
// * Input must be 2D
// * number of rows should equal number of columns (square matrix)
if cfg!(debug_assertions) {
Owner

debug_assertions is for debugging the compiler.
This is performing a user input check.

debug_assertions is for debugging the compiler. This is performing a user input check.
abdul124 force-pushed support_nalgebra from ea5f4fc698 to 8693cb8f6a 2024-07-25 12:42:14 +08:00 Compare
Author
Collaborator

Adds implementation of:

  1. np_dot: Unlike numpy where dot product can be used for multiplying matrices, the current implementation is limited to 1D case
  2. np_linalg_matmul: Same as np_matmul implemented by @ symbol currently but handles ValueErrors for incompatible matrices
  3. np_linalg_cholesky
  4. np_linalg_qr: The resulting matrices are slightly different than the ones from numpy, but does give a valid decomposition
  5. np_linalg_svd: The resulting matrices are slightly different than the ones from numpy, but does give a valid decomposition
  6. np_linalg_inv
  7. np_linalg_pinv
  8. sp_linalg_lu
  9. sp_linalg_schur: The resulting matrices are slightly different than the ones from numpy, but does give a valid decomposition
  10. sp_linalg_hessenberg: The resulting matrices are slightly different than the ones from numpy, but does give a valid decomposition

With the exception of np_dot all other operations work on 2D NDArray only. Other numpy and scipy functions like max, min, transpose, reshape, lu_solve etc. can all be similarly implemented using the corresponding functions in nalgebra but will be limited to 1D or 2D arrays

Adds implementation of: 1. `np_dot`: Unlike `numpy` where dot product can be used for multiplying matrices, the current implementation is limited to 1D case 2. `np_linalg_matmul`: Same as `np_matmul` implemented by `@` symbol currently but handles ValueErrors for incompatible matrices 3. `np_linalg_cholesky` 4. `np_linalg_qr`: The resulting matrices are slightly different than the ones from numpy, but does give a valid decomposition 5. `np_linalg_svd`: The resulting matrices are slightly different than the ones from numpy, but does give a valid decomposition 6. `np_linalg_inv` 7. `np_linalg_pinv` 8. `sp_linalg_lu` 9. `sp_linalg_schur`: The resulting matrices are slightly different than the ones from numpy, but does give a valid decomposition 10. `sp_linalg_hessenberg`: The resulting matrices are slightly different than the ones from numpy, but does give a valid decomposition With the exception of `np_dot` all other operations work on 2D `NDArray` only. Other `numpy` and `scipy` functions like `max`, `min`, `transpose`, `reshape`, `lu_solve` etc. can all be similarly implemented using the corresponding functions in `nalgebra` but will be limited to 1D or 2D arrays
abdul124 changed title from WIP: add nalgebra::linalg methods to add nalgebra::linalg methods 2024-07-25 12:57:28 +08:00
abdul124 requested review from sb10q 2024-07-25 12:58:09 +08:00
Owner

the current implementation is limited to 1D case

Why?

> the current implementation is limited to 1D case Why?
Owner

The resulting matrices are slightly different

What does that means exactly? And why?

> The resulting matrices are slightly different What does that means exactly? And why?
sb10q reviewed 2024-07-25 20:57:30 +08:00
@ -1838,0 +1883,4 @@
let (BasicTypeEnum::FloatType(_), BasicTypeEnum::FloatType(_)) = (n1_elem_ty, n2_elem_ty)
else {
unimplemented!("{FN_NAME} operates on float type NdArrays only");
Owner

Will this just crash the compiler if the user tries to do it on integer matrices?
See how the other compiler user error reporting is done.

Will this just crash the compiler if the user tries to do it on integer matrices? See how the other compiler user error reporting is done.
Author
Collaborator

For now, I have specified the numpy element type in linalg_externfns/src/lib.rs to be f64. We can extend this to support i32 or i64 type as well but will need dedicated functions to handle different types.

For now, I have specified the numpy element type in `linalg_externfns/src/lib.rs` to be `f64`. We can extend this to support `i32` or `i64` type as well but will need dedicated functions to handle different types.
Owner

I think limiting to float is fine. But the compiler should not crash if the user passes integer, and report the error correctly instead.

I think limiting to float is fine. But the compiler should not crash if the user passes integer, and report the error correctly instead.
sb10q reviewed 2024-07-25 20:58:27 +08:00
@ -1838,0 +2033,4 @@
}
}
/// Invokes the `np_linalg_svd` using `nalgebra` crate
Owner

What crate is used is a firmware implementation detail. Remove this comment and remove any code that goes along that line.

What crate is used is a firmware implementation detail. Remove this comment and remove any code that goes along that line.
sb10q reviewed 2024-07-25 21:00:21 +08:00
@ -0,0 +31,4 @@
raise_exn!($name, $fn_name, $message, 0, 0, 0)
}};
}
pub struct InputMatrix {
Owner

code formatting

code formatting
sb10q reviewed 2024-07-25 21:01:23 +08:00
@ -0,0 +8,4 @@
use core::slice;
use nalgebra::DMatrix;
macro_rules! raise_exn {
Owner

We don't support exceptions in the host demo and we don't plan to. Just call abort(), assert() and similar. Please keep everything as simple as possible.

We don't support exceptions in the host demo and we don't plan to. Just call abort(), assert() and similar. Please keep everything as simple as possible.
Author
Collaborator

Added this in-line with the firmware, so the runtime behavior can be somewhat replicated making it simpler when moving lib.rs to artiq-zynq or artiq system. Will replace the exceptions with a panic statement in standalone then.

Added this in-line with the firmware, so the runtime behavior can be somewhat replicated making it simpler when moving `lib.rs` to artiq-zynq or artiq system. Will replace the exceptions with a `panic` statement in standalone then.
sb10q reviewed 2024-07-25 21:02:32 +08:00
@ -0,0 +30,4 @@
}
}
pub unsafe fn raise(exception: *const Exception) -> ! {
Owner

Does not belong in this PR
Does not belong in a "linalg" crate
Not necessary for the host demo

Does not belong in this PR Does not belong in a "linalg" crate Not necessary for the host demo
sb10q reviewed 2024-07-25 21:03:36 +08:00
@ -45,2 +45,4 @@
linalg_externfns=../../target/debug/deps/liblinalg_externfns-?*.a
elif [ -e ../../target/release/nac3standalone ]; then
nac3standalone=../../target/release/nac3standalone
linalg_externfns=../../target/release/deps/liblinalg_externfns-?*.a
Owner

Why the globbing?

Why the globbing?
Author
Collaborator

Doing cargo build in standalone directory builds linalg_externfns as a dependency and adds a hash (I think its a hash) with its name.

Doing `cargo build` in standalone directory builds `linalg_externfns` as a dependency and adds a hash (I think its a hash) with its name.
Owner

Are you sure you're not using the wrong files? IIRC cargo eventually gives you one single .a with a stable name when building a static lib.

Are you sure you're not using the wrong files? IIRC cargo eventually gives you one single .a with a stable name when building a static lib.
Owner

Not resolved?

Not resolved?
Author
Collaborator

Globbing was needed when linalg was used as dependency of standalone (cargo puts dependencies separately with a hash in front of their names). After delegating the compilation to run_demo.sh, there is no longer any need for globbing.

Globbing was needed when linalg was used as dependency of standalone (cargo puts dependencies separately with a hash in front of their names). After delegating the compilation to `run_demo.sh`, there is no longer any need for globbing.
sb10q reviewed 2024-07-25 21:04:33 +08:00
@ -17,9 +17,7 @@ demo="$1"
echo -n "Checking $demo... "
./interpret_demo.py "$demo" > interpreted.log
./run_demo.sh --out run.log "${nac3args[@]}" "$demo"
./run_demo.sh --lli --out run_lli.log "${nac3args[@]}" "$demo"
Owner

Removing LLI should (1) be done more thoroughly, we don't want to drag dead code along (2) be a separate PR.

Removing LLI should (1) be done more thoroughly, we don't want to drag dead code along (2) be a separate PR.
Author
Collaborator

Since we can't really link a static library with lli it won't be able to run the new functions and the tests will fail. Generating DSO and linking using that should work though.

Since we can't really link a static library with `lli` it won't be able to run the new functions and the tests will fail. Generating DSO and linking using that should work though.
Owner

Yes, let's just remove the LLI tests entirely.

Yes, let's just remove the LLI tests entirely.
Author
Collaborator

the current implementation is limited to 1D case

Why?

When performing dot product on 1D vectors, output is a single float while a 2D matrix needs to be allocated with 2D inputs. The return type in two cases was different. The numpy documentation recommends using matmul for 2D case anyways and since it was already implemented, I though it would be fine to have np_dot calculate inner product alone.

The resulting matrices are slightly different

What does that means exactly? And why?

Matrix decompositions that give two matrices and don't have unique decompositions had some of their entries oppositely signed. An issue was raised on the nalgebra for QR discrepancies (https://github.com/dimforge/nalgebra/issues/735) but their fix doesn't really work. I am not sure about the reason for the discrepancy as usually one of the rows (and corresponding column of the other matrix) will have its sign reversed. The decomposition is still valid though, so I don't think it will pose an issue with actual usage.

> > the current implementation is limited to 1D case > > Why? When performing dot product on 1D vectors, output is a single float while a 2D matrix needs to be allocated with 2D inputs. The return type in two cases was different. The `numpy` documentation recommends using `matmul` for 2D case anyways and since it was already implemented, I though it would be fine to have `np_dot` calculate inner product alone. > > The resulting matrices are slightly different > > What does that means exactly? And why? Matrix decompositions that give two matrices and don't have unique decompositions had some of their entries oppositely signed. An issue was raised on the `nalgebra` for QR discrepancies (https://github.com/dimforge/nalgebra/issues/735) but their fix doesn't really work. I am not sure about the reason for the discrepancy as usually one of the rows (and corresponding column of the other matrix) will have its sign reversed. The decomposition is still valid though, so I don't think it will pose an issue with actual usage.
abdul124 force-pushed support_nalgebra from 8693cb8f6a to 688e85d13c 2024-07-29 17:34:51 +08:00 Compare
abdul124 reviewed 2024-07-29 17:39:06 +08:00
@ -63,0 +66,4 @@
# Compile linalg crate to provide functions compatible with i386 architecture
if [ ! -f ../../target/i686-unknown-linux-gnu/release/liblinalg.a ]; then
cd linalg && nix-shell -p rustup --command "RUSTFLAGS=\"-C target-cpu=i386 -C target-feature=+sse2\" cargo build -q --release --target=i686-unknown-linux-gnu" && cd ..
Author
Collaborator

Since we are testing on i386 as well, had to build an alternative static lib for the new target for linking.

Since we are testing on i386 as well, had to build an alternative static lib for the new target for linking.
Owner

I don't think you need to use rustup for that. Additionally, rustup will not work on Hydra.

If this is fixing rustc whining about "you need to use nightly toolchain for bla bla bla", just shut it up with RUSTC_BOOTSTRAP=1.

I don't think you need to use rustup for that. Additionally, rustup will not work on Hydra. If this is fixing rustc whining about "you need to use nightly toolchain for bla bla bla", just shut it up with RUSTC_BOOTSTRAP=1.
Author
Collaborator

I don't think you need to use rustup for that. Additionally, rustup will not work on Hydra.

Only the x86_64 version libstd was provided by default in the toolchain, so used rustup to build the static lib for i386 architecture.

If this is fixing rustc whining about "you need to use nightly toolchain for bla bla bla", just shut it up with RUSTC_BOOTSTRAP=1.

Tried building the libstd using -Zbuild-std, but that still required rust-src from rustup. I didn't find a workaround that could avoid using rustup, so added the compiled static lib file for both architectures along this PR.

> I don't think you need to use rustup for that. Additionally, rustup will not work on Hydra. Only the `x86_64` version libstd was provided by default in the toolchain, so used `rustup` to build the static lib for i386 architecture. > If this is fixing rustc whining about "you need to use nightly toolchain for bla bla bla", just shut it up with RUSTC_BOOTSTRAP=1. Tried building the libstd using `-Zbuild-std`, but that still required `rust-src` from rustup. I didn't find a workaround that could avoid using `rustup`, so added the compiled static lib file for both architectures along this PR.
Owner

Look here for a full workaround:
https://git.m-labs.hk/M-Labs/kirdy/src/branch/master/BSD.md

Committing binaries to the repository is not acceptable.

Look here for a full workaround: https://git.m-labs.hk/M-Labs/kirdy/src/branch/master/BSD.md Committing binaries to the repository is not acceptable.
Author
Collaborator

Moved linalg functions under nac3standalone/demo and updated the comments and error reporting.

Moved `linalg` functions under `nac3standalone/demo` and updated the comments and error reporting.
abdul124 force-pushed support_nalgebra from 08a5b74ae6 to 4a6845dac6 2024-07-31 13:27:00 +08:00 Compare
abdul124 added 2 commits 2024-07-31 18:03:18 +08:00
Author
Collaborator

Reimplemented simple functions (np_transpose and np_dot) using LLVM IR and added additional functions (np_linalg_matrix_power, np_linalg_det).

Reimplemented simple functions (`np_transpose` and `np_dot`) using LLVM IR and added additional functions (`np_linalg_matrix_power`, `np_linalg_det`).
sb10q reviewed 2024-08-01 18:06:24 +08:00
@ -1839,0 +1867,4 @@
out_ptr
}
/// Invokes the `np_linalg_matmul` linalg function
Owner

remove

remove
abdul124 marked this conversation as resolved
sb10q reviewed 2024-08-01 18:08:30 +08:00
@ -102,0 +114,4 @@
FunNpLinalgDet,
FunSpLinalgLu,
FunSpLinalgSchur,
FunSpLinalgHessenberg,
Owner

Bit weird to have those complicated matrix ops before simple functions like round() in the list.

Bit weird to have those complicated matrix ops before simple functions like round() in the list.
Owner

Probably better addressed in another PR though.

Probably better addressed in another PR though.
sb10q reviewed 2024-08-01 18:09:40 +08:00
@ -102,0 +104,4 @@
// Linalg functions
FunNpDot,
FunNpLinalgMatmul,
Owner

I think having only @ is fine.
linalg.matmul is probably from before Python introduced @.

I think having only ``@`` is fine. linalg.matmul is probably from before Python introduced ``@``.
sb10q reviewed 2024-08-01 18:10:59 +08:00
@ -8,6 +8,7 @@ edition = "2021"
parking_lot = "0.12"
nac3parser = { path = "../nac3parser" }
nac3core = { path = "../nac3core" }
linalg = { path = "./demo/linalg" }
Owner

This is part of the demo and the demo only. Shouldn't the demo runner build that part, instead of building it when nac3standalone is built?

This is part of the demo and the demo only. Shouldn't the demo runner build that part, instead of building it when nac3standalone is built?
Owner

Just like the demo runner is already building the glue C code.

Just like the demo runner is already building the glue C code.
Author
Collaborator

This is part of the demo and the demo only. Shouldn't the demo runner build that part, instead of building it when nac3standalone is built?

Will delegate its compilation to run_demo.sh similar to the current approach when testing with -i386.

> This is part of the demo and the demo only. Shouldn't the demo runner build that part, instead of building it when nac3standalone is built? Will delegate its compilation to `run_demo.sh` similar to the current approach when testing with -i386.
sb10q reviewed 2024-08-01 18:11:51 +08:00
@ -5,6 +5,7 @@ import importlib.util
import importlib.machinery
import math
import numpy as np
import scipy as sp
Owner

Move one line below.

Move one line below.
Owner

To keep the numpy imports together.

To keep the numpy imports together.
abdul124 force-pushed support_nalgebra from f954887acf to 2237137f1a 2024-08-02 13:12:30 +08:00 Compare
abdul124 force-pushed support_nalgebra from 2237137f1a to 63d2b49b09 2024-08-05 11:52:03 +08:00 Compare
Author
Collaborator

The standalone tests with -i386 will fail currently as linalg function compilation to i386 is left out (

cd linalg && cargo build --release --target i686-unknown-linux-gnu -q && cd ..
).

The standalone tests with -i386 will fail currently as `linalg` function compilation to `i386` is left out (https://git.m-labs.hk/M-Labs/nac3/src/commit/63d2b49b09aab6eace463b20e99ce8b5c19db50b/nac3standalone/demo/run_demo.sh#L65).
sb10q reviewed 2024-08-05 16:42:35 +08:00
@ -64,2 +64,2 @@
clang -m32 -c -std=gnu11 -Wall -Wextra -O3 -msse2 -o demo.o demo.c
clang -m32 -lm -Wl,--no-warn-search-mismatch -o demo module.o demo.o
cd linalg && cargo build --release --target i686-unknown-linux-gnu -q && cd ..
Owner

indentation

indentation
Owner

But nevermind I'll fix it

But nevermind I'll fix it
sb10q added 1 commit 2024-08-05 19:04:57 +08:00
sb10q merged commit 669c6aca6b into master 2024-08-05 19:05:26 +08:00
sb10q deleted branch support_nalgebra 2024-08-05 19:05:26 +08:00
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/nac3#478
No description provided.