Skip to content

Commit

Permalink
fix: address @kcelia review
Browse files Browse the repository at this point in the history
  • Loading branch information
robinstraub committed Apr 18, 2023
1 parent 5f8fd00 commit 9276e48
Showing 1 changed file with 462 additions and 154 deletions.
616 changes: 462 additions & 154 deletions concrete-ml/svm.ipynb

Large diffs are not rendered by default.

1 comment on commit 9276e48

@kcelia
Copy link

@kcelia kcelia commented on 9276e48 Apr 19, 2023

Choose a reason for hiding this comment

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

Hello @robinstraub,

I really like the new version, thanks for your work. I have few comments:

  • typos: "wether"
  • Some of our internal conventions:
    • Use present simple, nominative phrases, and avoid possessive pronouns as much as possible.
    • We start bullet points in lowercase.
    • We avoid using "let's."
  • Use "FHE simulation" instead of "virtual library" (in v1.0, we decided to remove this naming).
  • In v1.0, the maximum bit size becomes 16 bits, not 8 bits.
  • Possible rephrasing: In this tutorial, we show how to create, train, and evaluate a Support Vector Machine (SVM) model using Concrete-ML library for a classification task.
    (You talk about the Concrete-ML library in the next cell.)
  • Possible rephrasing: In this tutorial, we use the pulsar star data-set to determine whether a neutron star can be classified as a pulsar star.
  • In the following cell, print the max bit-width of the circuit to the user: circuit.graph.maximum_integer_bit_width()because later in the text, you say it's 8 bits.
    # A circuit needs to be compiled to enable FHE execution
    circuit = svm_concrete.compile(X_train)
    # Now that a circuit is compiled, the svm_concrete can predict value with FHE
    y_pred = svm_concrete.predict(X_test, fhe="execute")
    accuracy = accuracy_score(y_test, y_pred)
    # print the accuracy
    print(f"FHE Accuracy: {accuracy:.4f}")
    
  • Possible rephrasing: The execution speed can be slower in Concrete-ML, especially during compilation and FHE inference phases, because enabling FHE operations uses more resources than regular inference on plain data. However, the speed can be improved by decreasing the precision of the data and model's weights thanks to the n_bits parameter. But, depending on the project, there is a trade-off between a slower but more accurate model and a faster but less accurate model.
  • Possible rephrasing: Use FHE simulation mode to speed up the development process by finding the most interesting hyper-parameters.
  • Possible rephrasing: Setting up a model in Concrete-ML requires some additional work compared to standard models. For instance, users must select the quantization bit-width for both the model's weight and input data, which can be complex and time-consuming while using real FHE inference. However, Concrete-ML provides an FHE simulation mode that allows users to identify optimal hyper-parameters with the best trade-off between latency and performance.
  • In Concrete-ML, training is done in the clear using scikit-learn pipeline. For inference, we propose two ways:
    • Inference without encryption:
      • Input data and model weights are quantized, and there is no FHE operation.
      • It's not costly, but you may notice a slight decrease in performance due to quantization.
    • Inference with encryption:
      • Input data and model weights are quantized with FHE operation.
      • It's very costly, but the result of the inference is the same as 1.
      • That's where we use the FHE simulation mode to find the best optimal hyperparameter that offers a good compromise between latency and performance. In other words, you allow to decrease your accuracy a little bit but you speed up computation.Hence it's not the best hyperparameter like we used to do in scikit learn but the optimal one.
        For compilation, you can use a representative subset of the training set, it will speed up a bit the compilation time. But it's already fast.

I propose to remove this cell:

# compute the accuracy of a n_bit quantized linear ranging from 2 to 8 bits
for n_bits in range(2, 9):
    svm_concrete = ConcreteLinearSVC(n_bits)
    svm_concrete.fit(X_train, y_train)
  y_pred = svm_concrete.predict(X_test)
  accuracy = accuracy_score(y_test, y_pred)
  print(f"{n_bits} Bits Quantized Accuracy: {accuracy:.4f}")

In the next cell, where you show grid search, instead of using the best hyper-parameter, show all the results of your grid search in a table (for example with pandas), then highlight the optimal hyperparameter that offers a good compromise between latency and performance. For example:
2 Bits Quantized Accuracy: 0.9525
3 Bits Quantized Accuracy: 0.9745
4 Bits Quantized Accuracy: 0.9745
5 Bits Quantized Accuracy: 0.9761
6 Bits Quantized Accuracy: 0.9761
7 Bits Quantized Accuracy: 0.9769
8 Bits Quantized Accuracy: 0.9765

Here, n_bit = 3 is the best compromise between latency and performance, so you could highlight the corresponding row in a different color using pandas.

Thanks again !

Please sign in to comment.