removing ipex, and fixing multigpu detection help from @AbrarFaiyad - #1522
removing ipex, and fixing multigpu detection help from @AbrarFaiyad#1522alvarovm wants to merge 2 commits into
Conversation
Bugbot couldn't runBugbot is not enabled for your user on this team. Ask your team administrator to increase your team's hard limit for Bugbot seats or add you to the allowlist in the Cursor dashboard. |
1 similar comment
Bugbot couldn't runBugbot is not enabled for your user on this team. Ask your team administrator to increase your team's hard limit for Bugbot seats or add you to the allowlist in the Cursor dashboard. |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bc9674a. Configure here.
| logging.error( | ||
| "Intel Extension for PyTorch not found, but XPU device was specified. " | ||
| "Please install it to use XPU device." | ||
| ) |
There was a problem hiding this comment.
Evaluation DDP lacks XPU fix
High Severity
After training, distributed post-training evaluation rebuilds DDP with device_ids=[local_rank] and skips the XPU pinning added earlier. On Intel layouts where each process sees one tile and local_rank can exceed the visible device index, this can hit the same DDP overflow that the new _ddp_dev logic avoids during the main training loop.
Reviewed by Cursor Bugbot for commit bc9674a. Configure here.
There was a problem hiding this comment.
This is fine. Here model is not distributed.
Removed check for XPU device availability in PyTorch in run_train.py. This is already checked later at init_device() in mace/tools/torch_tools.py#L67
alvarovm
left a comment
alvarovm
left a comment
There was a problem hiding this comment.
Removed useless warnings. Reviewed code.
aacostadiaz
left a comment
aacostadiaz
left a comment
There was a problem hiding this comment.
Thanks for this cleanup! I've left a few comments below.
Since we don't have Intel hardware in CI, we can't test any of the XPU paths ourselves. Could you re-run a multi-node job on your cluster once the comments are addressed and confirm it works end to end?
| # after param.requires_grad = False was called before evaluating stage-one model | ||
| for param in model.parameters(): | ||
| param.requires_grad = True | ||
| distributed_model = DDP(model, device_ids=[local_rank]) |
There was a problem hiding this comment.
There's a second DDP here in the final evaluation loop that still uses the raw local_rank. I think this is the line the bot was actually referring to in https://github.com/ACEsuit/mace/pull/1522/changes/BASE..bc9674a8dfaf1310cec5c53f7e4e2265abae9370#r3574263042
distributed_model = DDP(model, device_ids=[_ddp_dev])There was a problem hiding this comment.
torch >2.8 DDP supports Intel CCL back-end.
| update_keyspec_from_kwargs(args.key_specification, vars(args)) | ||
|
|
||
| if args.device == "xpu": | ||
| try: |
There was a problem hiding this comment.
I think removing this import breaks distributed XPU. Importing oneccl_bindings_for_pytorch was what registered the "ccl" backend and distributed_tools.py still calls init_process_group(backend="ccl") for xpu
There was a problem hiding this comment.
The oneccl_bindings_for_pytorch is also retired per intel see https://github.com/intel/torch-ccl
3 participants
We propose to remove IPEX modules for Intel XPU training and ASE calculator. IPEX was retired (archived) by Intel in favor of XPU support within Pytorch >=2.8.
We also fixed the support for multigpu and multinode training on large Intel GPU clusters.
Note
Medium Risk
Touches distributed training device mapping on XPU clusters; behavior change for users who still relied on IPEX, but scope is limited to Intel XPU paths.
Overview
Removes Intel Extension for PyTorch (IPEX) from the ASE
MACECalculatorandrun_train.py, including allipex.optimizepaths, in favor of nativetorch.xpusupport in recent PyTorch.XPU startup no longer requires importing IPEX/oneCCL; it only logs when
--device=xpuis set buttorch.xpuis missing. Distributed XPU now clampstorch.xpu.set_devicewhenlocal_rankexceeds the visible device count, and DDP usesdevice_ids=[0]when each process sees a single XPU (e.g. underZE_AFFINITY_MASK), avoiding overflow errors from invalidlocal_rankdevice IDs.Reviewed by Cursor Bugbot for commit bc9674a. Configure here.