-
Notifications
You must be signed in to change notification settings - Fork 732
Fix Neutron backend API compatibility and multiprocessing fallback #15885
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15885
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New Failure, 2 Unrelated FailuresAs of commit 8aff4aa with merge base 213d24c ( NEW FAILURE - The following job has failed:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
50bffbb to
b5757bb
Compare
…ytorch#15885) Summary: This fixes two issues preventing the Neutron backend from building in sandcastle/build environments: 1. **NeutronAtenPassManager API compatibility**: The `NeutronAtenPassManager` class was updated to require a `neutron_target_spec` parameter, but the call site wasn't updated. This caused a `TypeError` at runtime. Fixed by passing the already-available `neutron_target_spec` to the pass manager constructor. 2. **Multiprocessing fallback**: The Neutron converter uses multiprocessing for isolation when converting models, but this fails in restricted build environments (sandcastle) with an `EOFError` when trying to create a multiprocessing Manager. Added a try/except block to catch `EOFError` and `OSError` and fall back to direct execution of the converter when multiprocessing is unavailable. This maintains isolation in normal environments while allowing builds to succeed in restricted ones. Both issues were pre-existing on trunk and not caused by recent changes. Differential Revision: D87252895
|
@apullin , LGTM, although, don't see the change related to
I checked the executorch OSS code base and did not find the above mentioned API compatibility break. Is is something in your internal code base? |
This is on an internal SDK, yes. My first time syncing between, so I should have labeled that. |
…ytorch#15885) Summary: This fixes two issues preventing the Neutron backend from building in sandcastle/build environments: 1. **NeutronAtenPassManager API compatibility**: The `NeutronAtenPassManager` class was updated to require a `neutron_target_spec` parameter, but the call site wasn't updated. This caused a `TypeError` at runtime. Fixed by passing the already-available `neutron_target_spec` to the pass manager constructor. 2. **Multiprocessing fallback**: The Neutron converter uses multiprocessing for isolation when converting models, but this fails in restricted build environments (sandcastle) with an `EOFError` when trying to create a multiprocessing Manager. Added a try/except block to catch `EOFError` and `OSError` and fall back to direct execution of the converter when multiprocessing is unavailable. This maintains isolation in normal environments while allowing builds to succeed in restricted ones. Both issues were pre-existing on trunk and not caused by recent changes. Differential Revision: D87252895
b5757bb to
8aff4aa
Compare
Summary:
This fixes two issues preventing the Neutron backend from building in sandcastle/build environments:
NeutronAtenPassManager API compatibility: The
NeutronAtenPassManagerclass was updated to require aneutron_target_specparameter, but the call site wasn't updated. This caused aTypeErrorat runtime. Fixed by passing the already-availableneutron_target_specto the pass manager constructor.Multiprocessing fallback: The Neutron converter uses multiprocessing for isolation when converting models, but this fails in restricted build environments (sandcastle) with an
EOFErrorwhen trying to create a multiprocessing Manager. Added a try/except block to catchEOFErrorandOSErrorand fall back to direct execution of the converter when multiprocessing is unavailable. This maintains isolation in normal environments while allowing builds to succeed in restricted ones.Both issues were pre-existing on trunk and not caused by recent changes.
Differential Revision: D87252895