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.

Sign up for GitHub

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

Jump to bottom

Feat/replace rounding by truncate [ON HOLD - NOT URGENT] #472

Closed
wants to merge 75 commits into from
Closed

Feat/replace rounding by truncate [ON HOLD - NOT URGENT] #472

wants to merge 75 commits into from

Conversation

kcelia
Copy link
Collaborator

@kcelia kcelia commented Jan 29, 2024

I replaced rounding with truncate.

This PR is not urgent.

what's left to do:

  • clean up the comments
  • remove a fixme
  • test truncate vs rounding speedup

kcelia and others added 30 commits December 11, 2023 16:55
@kcelia
chore: Version 1
a3649b6 
- Set manually number of lsb to be removed in ops_impl.py for comparison
  functions
- Confirm that rounding improves the execution time for tree-based models
  (see experiements in fhenet-experiments
@kcelia
chore: rebase on main
e12f13a
@kcelia
chore: add new rounded operators
d88d603
@kcelia
chore: add rounded operators to ONNX_OPS_TO_NUMPY_IMPL_BOOL and remov…
e630ab5 
…e lsbs to remove
@kcelia
chore: update convert.py
3ea13c8
@jfrery @kcelia
chore: add conversion to our rounded ops in onnx graph for trees
de51201
@jfrery @kcelia
chore: update conversion to rounded ops to use list of lsbs
d79127f
@kcelia
chore: fix lsb to remove according to op comparison
dddead6
@kcelia
chore: reconcial with Jordan's version
6a33e62 
disable check_model for the moment
remove redundant check_model
update formulas
@kcelia
chore: update
9bf30ce
@kcelia
chore: fix dump tests
c0107a6
@kcelia
chore: rename variable
6c9a475
@kcelia
chore: restore previous version
3d858fe
@kcelia
chore: remove replace_operator_with_rounded_version function
aac906f
@kcelia
chore: update
992f230
@kcelia
chore: add comments
0243dc7
@kcelia
chore: fix divergence in graphs after serialization
dc08fb0
@kcelia
chore: update
78f3185
@kcelia
chore: coverage
4f9859b
@kcelia
chore: update lsbs 2
251e91d
@kcelia
chore: update
4d68c95
@kcelia
chore: update
d186ec8
@kcelia
chore: update with new version
c1fcf5f
@kcelia
chore: update lsb computation
012ddb6
@kcelia
chore: fix assert c_r1 < c_r0 + 2
399ccc7
@kcelia
chore: update serialized test
6956996
@kcelia
chore: version for Jordan
a9b2b06
@kcelia
chore: version with MSB=1
ffa4577
@kcelia
chore: update
9031531
@kcelia
chore: remove rounding in serialization
71b56b2
@kcelia kcelia requested a review from a team as a code owner January 29, 2024 11:38
@cla-bot cla-bot bot added the cla-signed label Jan 29, 2024
@kcelia kcelia force-pushed the feat/replace_rounding_by_truncate branch from e43a231 to 95f008f Compare January 29, 2024 12:41
@kcelia kcelia marked this pull request as draft January 29, 2024 14:15
@kcelia kcelia force-pushed the feat/replace_rounding_by_truncate branch from 95f008f to 127c4e0 Compare January 29, 2024 14:57
@kcelia
Copy link
Collaborator Author

kcelia commented Jan 30, 2024

For the truncate vs. rounding speedup benchmark:

I adjusted the rounding method by removing the half to fairly compare it with truncate.

However, during certain executions, I encountered an error linked to Rust:

thread '<unnamed>' panicked at concrete-optimizer/src/optimization/dag/multi_parameters/symbolic_variance.rs:79:9:
assertion failed: new.coeffs[index] == 0.0
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread '<unnamed>' panicked at /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cxx-1.0.97/src/unwind.rs:37:9:
panic in ffi function concrete_optimizer_cpp::ffi::OperationDag::optimize_multi, aborting.
stack backtrace:
   0:     0x7f9702e1c54c - std::backtrace_rs::backtrace::libunwind::trace::ha637c64ce894333a
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/../../backtrace/src/backtrace/libunwind.rs:104:5
   1:     0x7f9702e1c54c - std::backtrace_rs::backtrace::trace_unsynchronized::h47f62dea28e0c88d
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x7f9702e1c54c - std::sys_common::backtrace::_print_fmt::h9eef0abe20ede486
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/sys_common/backtrace.rs:67:5
   3:     0x7f9702e1c54c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hed7f999df88cc644
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x7f9702e6cb10 - core::fmt::rt::Argument::fmt::h1539a9308b8d058d
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/fmt/rt.rs:142:9
   5:     0x7f9702e6cb10 - core::fmt::write::h3a39390d8560d9c9
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/fmt/mod.rs:1120:17
   6:     0x7f9702e11baf - std::io::Write::write_fmt::h5fc9997dfe05f882
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/io/mod.rs:1762:15
   7:     0x7f9702e1c334 - std::sys_common::backtrace::_print::h894006fb5c6f3d45
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x7f9702e1c334 - std::sys_common::backtrace::print::h23a2d212c6fff936
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x7f9702e1efc7 - std::panicking::default_hook::{{closure}}::h8a1d2ee00185001a
  10:     0x7f9702e1ed2f - std::panicking::default_hook::h6038f2eba384e475
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/panicking.rs:292:9
  11:     0x7f9702e1f508 - std::panicking::rust_panic_with_hook::h2b5517d590cab22e
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/panicking.rs:779:13
  12:     0x7f9702e1f3ee - std::panicking::begin_panic_handler::{{closure}}::h233112c06e0ef43e
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/panicking.rs:657:13
  13:     0x7f9702e1ca16 - std::sys_common::backtrace::__rust_end_short_backtrace::h6e893f24d7ebbff8
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/sys_common/backtrace.rs:170:18
  14:     0x7f9702e1f152 - rust_begin_unwind
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/panicking.rs:645:5
  15:     0x7f96fe6f3125 - core::panicking::panic_fmt::hbf0e066aabfa482c
                               at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/panicking.rs:72:14
  16:     0x7f96fe6ee498 - <cxx::unwind::Guard as core::ops::drop::Drop>::drop::hda87c798fb938e77
  17:     0x7f9702d903ac - concrete_optimizer$cxxbridge1$OperationDag$optimize_multi
  18:     0x7f9702d8ca75 - _ZNK18concrete_optimizer12OperationDag14optimize_multiENS_7OptionsE
  19:     0x7f97005cf607 - _ZN4mlir12concretelang19getDagMultiSolutionERN4rust10cxxbridge13BoxIN18concrete_optimizer12OperationDagEEENS0_9optimizer6ConfigE.localalias
  20:     0x7f97005d25b5 - _ZN4mlir12concretelang11getSolutionERNS0_9optimizer11DescriptionERNS0_19CompilationFeedbackENS1_6ConfigE
  21:     0x7f97005c58a9 - _ZN4mlir12concretelang14CompilerEngine22determineFHEParametersERNS1_17CompilationResultE.localalias
  22:     0x7f97005c6653 - _ZN4mlir12concretelang14CompilerEngine7compileENS_8ModuleOpENS1_6TargetESt8optionalISt10shared_ptrINS1_7LibraryEEE.localalias
  23:     0x7f97005cac28 - _ZN4mlir12concretelang21compileModuleOrSourceINS_8ModuleOpEEEN4llvm8ExpectedINS0_14CompilerEngine7LibraryEEEPS5_T_NSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESF_bbbb
  24:     0x7f97005cb1c4 - _ZN4mlir12concretelang14CompilerEngine7compileENS_8ModuleOpENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES8_bbbb
  25:     0x7f96fe7cb0fd - _ZN4mlir12concretelang14LibrarySupport7compileERNS_8ModuleOpERSt10shared_ptrINS0_18CompilationContextEENS0_18CompilationOptionsE
  26:     0x7f96fe7d33f9 - _Z22library_compile_module17LibrarySupport_PyN4mlir8ModuleOpENS0_12concretelang18CompilationOptionsESt10shared_ptrINS2_18CompilationContextEE
  27:     0x7f96fb15ed35 - <unknown>
  28:     0x7f96fb1351db - <unknown>
  29:     0x7f96fb1139c7 - <unknown>
  30:           0x5223b7 - PyCFunction_Call
  31:           0x50ca93 - _PyObject_MakeTpCall
  32:           0x52213b - <unknown>
  33:           0x508491 - _PyEval_EvalFrameDefault
  34:           0x5027da - _PyEval_EvalCodeWithName
  35:           0x51499d - _PyFunction_Vectorcall
  36:           0x504066 - _PyEval_EvalFrameDefault
  37:           0x5027da - _PyEval_EvalCodeWithName
  38:           0x51499d - _PyFunction_Vectorcall
  39:           0x504b1f - _PyEval_EvalFrameDefault
  40:           0x5148ff - _PyFunction_Vectorcall
  41:           0x504066 - _PyEval_EvalFrameDefault
  42:           0x5027da - _PyEval_EvalCodeWithName
  43:           0x50c27c - _PyObject_FastCallDict
  44:           0x51ea03 - <unknown>
  45:           0x50caab - _PyObject_MakeTpCall
  46:           0x5083a7 - _PyEval_EvalFrameDefault
  47:           0x5027da - _PyEval_EvalCodeWithName
  48:           0x521f05 - <unknown>
  49:           0x504b1f - _PyEval_EvalFrameDefault
  50:           0x5027da - _PyEval_EvalCodeWithName
  51:           0x51499d - _PyFunction_Vectorcall
  52:           0x5247ce - PyObject_Call
  53:           0x505a66 - _PyEval_EvalFrameDefault
  54:           0x5027da - _PyEval_EvalCodeWithName
  55:           0x521f05 - <unknown>
  56:           0x504b1f - _PyEval_EvalFrameDefault
  57:           0x5148ff - _PyFunction_Vectorcall
  58:           0x503d43 - _PyEval_EvalFrameDefault
  59:           0x5148ff - _PyFunction_Vectorcall
  60:           0x503d43 - _PyEval_EvalFrameDefault
  61:           0x5027da - _PyEval_EvalCodeWithName
  62:           0x5d69a7 - PyEval_EvalCode
  63:           0x5f77c5 - <unknown>
  64:           0x5f67d3 - <unknown>
  65:           0x5f5876 - <unknown>
  66:           0x5f5527 - PyRun_SimpleFileExFlags
  67:           0x5f26c1 - Py_RunMain
  68:           0x5caa8d - Py_BytesMain
  69:     0x7f97e9b82d90 - __libc_start_call_main
                               at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  70:     0x7f97e9b82e40 - __libc_start_main_impl
                               at ./csu/../csu/libc-start.c:392:3
  71:           0x5ca985 - _start
  72:                0x0 - <unknown>
thread '<unnamed>' panicked at library/core/src/panicking.rs:144:5:
panic in a destructor during cleanup
thread caused non-unwinding panic. aborting.
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  libConcretelangBindingsPythonCAPI.so         0x00007f96fe9ed761 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 225
1  libConcretelangBindingsPythonCAPI.so         0x00007f96fe9eb174
2  libc.so.6                                    0x00007f97e9b9b520
3  libc.so.6                                    0x00007f97e9bef9fc pthread_kill + 300
4  libc.so.6                                    0x00007f97e9b9b476 raise + 22
5  libc.so.6                                    0x00007f97e9b817f3 abort + 211
6  libConcretelangBindingsPythonCAPI.so         0x00007f9702e2ab37 std::sys::unix::abort_internal::hc5747b989e1a1439 + 7
7  libConcretelangBindingsPythonCAPI.so         0x00007f9702e1f675 std::panicking::rust_panic_with_hook::h2b5517d590cab22e + 597
8  libConcretelangBindingsPythonCAPI.so         0x00007f9702e1f3b9
9  libConcretelangBindingsPythonCAPI.so         0x00007f9702e1ca16
10 libConcretelangBindingsPythonCAPI.so         0x00007f9702e1f152 rust_begin_unwind + 66
11 libConcretelangBindingsPythonCAPI.so         0x00007f96fe6f3168 core::panicking::panic_nounwind_fmt::h8767c3f35819d636 + 56
12 libConcretelangBindingsPythonCAPI.so         0x00007f96fe6f326c core::panicking::panic_nounwind_nobacktrace::hcbb327b90126879c + 76
13 libConcretelangBindingsPythonCAPI.so         0x00007f96fe6f3473 core::panicking::panic_in_cleanup::hd44bb2114362504e + 19
14 libConcretelangBindingsPythonCAPI.so         0x00007f9702d903bc concrete_optimizer$cxxbridge1$OperationDag$optimize_multi + 252
15 libConcretelangBindingsPythonCAPI.so         0x00007f9702d8ca75 concrete_optimizer::OperationDag::optimize_multi(concrete_optimizer::Options) const + 69
16 libConcretelangBindingsPythonCAPI.so         0x00007f97005cf607 mlir::concretelang::getDagMultiSolution(rust::cxxbridge1::Box<concrete_optimizer::OperationDag>&, mlir::concretelang::optimizer::Config) + 295
17 libConcretelangBindingsPythonCAPI.so         0x00007f97005d25b5 mlir::concretelang::getSolution(mlir::concretelang::optimizer::Description&, mlir::concretelang::CompilationFeedback&, mlir::concretelang::optimizer::Config) + 645
18 libConcretelangBindingsPythonCAPI.so         0x00007f97005c58a9 mlir::concretelang::CompilerEngine::determineFHEParameters(mlir::concretelang::CompilerEngine::CompilationResult&) + 1561
19 libConcretelangBindingsPythonCAPI.so         0x00007f97005c6653 mlir::concretelang::CompilerEngine::compile(mlir::ModuleOp, mlir::concretelang::CompilerEngine::Target, std::optional<std::shared_ptr<mlir::concretelang::CompilerEngine::Library>>) + 947
20 libConcretelangBindingsPythonCAPI.so         0x00007f97005cac28 llvm::Expected<mlir::concretelang::CompilerEngine::Library> mlir::concretelang::compileModuleOrSource<mlir::ModuleOp>(mlir::concretelang::CompilerEngine*, mlir::ModuleOp, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, bool, bool, bool, bool) + 824
21 libConcretelangBindingsPythonCAPI.so         0x00007f97005cb1c4 mlir::concretelang::CompilerEngine::compile(mlir::ModuleOp, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, bool, bool, bool, bool) + 196
22 libConcretelangBindingsPythonCAPI.so         0x00007f96fe7cb0fd
23 libConcretelangBindingsPythonCAPI.so         0x00007f96fe7d33f9 library_compile_module(LibrarySupport_Py, mlir::ModuleOp, mlir::concretelang::CompilationOptions, std::shared_ptr<mlir::concretelang::CompilationContext>) + 73
24 _concretelang.cpython-38-x86_64-linux-gnu.so 0x00007f96fb15ed35
25 _concretelang.cpython-38-x86_64-linux-gnu.so 0x00007f96fb1351db
26 _concretelang.cpython-38-x86_64-linux-gnu.so 0x00007f96fb1139c7
27 python                                       0x00000000005223b7 PyCFunction_Call + 87
28 python                                       0x000000000050ca93 _PyObject_MakeTpCall + 787
29 python                                       0x000000000052213b
30 python                                       0x0000000000508491 _PyEval_EvalFrameDefault + 19185
31 python                                       0x00000000005027da _PyEval_EvalCodeWithName + 762
32 python                                       0x000000000051499d _PyFunction_Vectorcall + 429
33 python                                       0x0000000000504066 _PyEval_EvalFrameDefault + 1734
34 python                                       0x00000000005027da _PyEval_EvalCodeWithName + 762
35 python                                       0x000000000051499d _PyFunction_Vectorcall + 429
36 python                                       0x0000000000504b1f _PyEval_EvalFrameDefault + 4479
37 python                                       0x00000000005148ff _PyFunction_Vectorcall + 271
38 python                                       0x0000000000504066 _PyEval_EvalFrameDefault + 1734
39 python                                       0x00000000005027da _PyEval_EvalCodeWithName + 762
40 python                                       0x000000000050c27c _PyObject_FastCallDict + 508
41 python                                       0x000000000051ea03
42 python                                       0x000000000050caab _PyObject_MakeTpCall + 811
43 python                                       0x00000000005083a7 _PyEval_EvalFrameDefault + 18951
44 python                                       0x00000000005027da _PyEval_EvalCodeWithName + 762
45 python                                       0x0000000000521f05
46 python                                       0x0000000000504b1f _PyEval_EvalFrameDefault + 4479
47 python                                       0x00000000005027da _PyEval_EvalCodeWithName + 762
48 python                                       0x000000000051499d _PyFunction_Vectorcall + 429
49 python                                       0x00000000005247ce PyObject_Call + 862
50 python                                       0x0000000000505a66 _PyEval_EvalFrameDefault + 8390
51 python                                       0x00000000005027da _PyEval_EvalCodeWithName + 762
52 python                                       0x0000000000521f05
53 python                                       0x0000000000504b1f _PyEval_EvalFrameDefault + 4479
54 python                                       0x00000000005148ff _PyFunction_Vectorcall + 271
55 python                                       0x0000000000503d43 _PyEval_EvalFrameDefault + 931
56 python                                       0x00000000005148ff _PyFunction_Vectorcall + 271
57 python                                       0x0000000000503d43 _PyEval_EvalFrameDefault + 931
58 python                                       0x00000000005027da _PyEval_EvalCodeWithName + 762
59 python                                       0x00000000005d69a7 PyEval_EvalCode + 39
60 python                                       0x00000000005f77c5
61 python                                       0x00000000005f67d3
62 python                                       0x00000000005f5876
63 python                                       0x00000000005f5527 PyRun_SimpleFileExFlags + 407
64 python                                       0x00000000005f26c1 Py_RunMain + 705
65 python                                       0x00000000005caa8d Py_BytesMain + 45
66 libc.so.6                                    0x00007f97e9b82d90
67 libc.so.6                                    0x00007f97e9b82e40 __libc_start_main + 128
68 python                                       0x00000000005ca985 _start + 37
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Aborted (core dumped)

@kcelia kcelia force-pushed the feat/replace_rounding_by_truncate branch from 127c4e0 to 4e2fcda Compare January 30, 2024 10:42
fd0r
fd0r reviewed Feb 21, 2024
View reviewed changes
src/concrete/ml/onnx/convert.py
@@ -255,7 +255,7 @@ def get_equivalent_numpy_forward_from_onnx(
def get_equivalent_numpy_forward_from_onnx_tree(
onnx_model: onnx.ModelProto,
check_model: bool = True,
lsbs_to_remove_for_trees: Optional[Tuple[int, int]] = None,
auto_truncate=None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure about the type of this, should it be a bool?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't take care of the typing in this PR (it's still in draft) and as I explained it's on hold.

auto_truncate is the concrete.fhe.auto_truncate object and not a bool, it can be None, if the feature is disabled.

In our test, we check that the feature is stable, i.e. we get the same result with and without truncate.
(in case the concrete team makes an update that changes the behavior of truncate).

@github-actions GitHub Actions
Copy link

Coverage failed ❌

Coverage details 

---------- coverage: platform linux, python 3.8.18-final-0 -----------
Name                                       Stmts   Miss  Cover   Missing
------------------------------------------------------------------------
src/concrete/ml/onnx/ops_impl.py             357      3    99%   925, 1101, 1173
src/concrete/ml/sklearn/tree_to_numpy.py     138     38    72%   406-504
------------------------------------------------------------------------
TOTAL                                       7127     41    99%

50 files skipped due to complete coverage.

@fd0r fd0r self-requested a review February 23, 2024 10:20
@andrei-stoian-zama
Copy link
Collaborator

Done in the @fd0r 's PR

@andrei-stoian-zama andrei-stoian-zama deleted the feat/replace_rounding_by_truncate branch October 17, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fd0r fd0r Awaiting requested review from fd0r

Assignees
No one assigned
Labels
cla-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kcelia @andrei-stoian-zama @fd0r @jfrery