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.

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

chore: add use_gpu for cifar finetuning #882

Merged
merged 5 commits into from
Sep 25, 2024
Merged

chore: add use_gpu for cifar finetuning #882

merged 5 commits into from
Sep 25, 2024

Conversation

kcelia
Copy link
Collaborator

@kcelia kcelia commented Sep 23, 2024

No description provided.

@kcelia kcelia self-assigned this Sep 23, 2024
@cla-bot cla-bot bot added the cla-signed label Sep 23, 2024
Copy link
Collaborator

@andrei-stoian-zama andrei-stoian-zama left a comment

Choose a reason for hiding this comment

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

Thanks, but you need to use the new CML GPU api instead of the Concrete one

Copy link
Collaborator

@andrei-stoian-zama andrei-stoian-zama left a comment

Choose a reason for hiding this comment

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

@kcelia Can you please confirm you can run these notebooks on your machine ?

@kcelia kcelia force-pushed the gpu_update_4608 branch 2 times, most recently from fc37f69 to 7fb522d Compare September 24, 2024 14:14
Comment on lines -56 to +62
model, images, n_bits, rounding_threshold_bits=None, fhe_mode="disable", use_gpu=False
model,
images,
n_bits,
rounding_threshold_bits=None,
fhe_mode="disable",
compilation_device="cpu",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I preferred use_gpu or just device=... to do the same as torch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"@jfrery, use_device might sound like a boolean type, but in the compilation functions, it's actually a string that can be either 'cpu' or 'gpu'.

The reason it's not simply named device is because the PyTorch device and the CML one may differ. This distinction helps differentiate between the device used for PyTorch training/evaluation and the one used for CML compilation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

device="cuda|cpu" is perfect yes but here we have compilation_device.

The reason it's not simply named device is because the PyTorch device and the CML one may differ.

Well as far as I know we try to stay close to the torch API. We did it for sklearn and the others. I don't see why now we would want to differ. device seems the right choice here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We assume that the compile_device may differ from the device used to train the model.

Additionally, on standard machines, setting compile_device="cpu" is currently faster than compile_device="cuda". So, it's better to keep them separate.

Copy link
Collaborator

@andrei-stoian-zama andrei-stoian-zama left a comment

Choose a reason for hiding this comment

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

one small error remains

@kcelia
Copy link
Collaborator Author

kcelia commented Sep 25, 2024

I tested on my machine, all the scripts are working

Copy link

⚠️ Known flaky tests have been rerun ⚠️

One or several tests initially failed but were identified as known flaky. tests. Therefore, they have been rerun and passed. See below for more details.

Failed tests details

Known flaky tests that initially failed:

  • tests/torch/test_compile_torch.py::test_compile_torch_or_onnx_conv_networks[True-True-CNN-relu]- tests/torch/test_compile_torch.py::test_compile_torch_or_onnx_conv_networks[False-True-CNN-relu]- tests/torch/test_compile_torch.py::test_compile_torch_or_onnx_conv_networks[False-True-CNN_grouped-relu]

Copy link

Coverage failed ❌

Coverage details

---------- coverage: platform linux, python 3.8.18-final-0 -----------
Name                                 Stmts   Miss  Cover   Missing
------------------------------------------------------------------
src/concrete/ml/onnx/onnx_utils.py      59     10    83%   594-620
------------------------------------------------------------------
TOTAL                                 8138     10    99%

59 files skipped due to complete coverage.

@kcelia kcelia marked this pull request as ready for review September 25, 2024 13:57
@kcelia kcelia requested a review from a team as a code owner September 25, 2024 13:57
@kcelia kcelia merged commit b37ceed into main Sep 25, 2024
17 of 18 checks passed
@kcelia kcelia deleted the gpu_update_4608 branch September 25, 2024 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants