Skip to content

[20/n] tensor engine, more refactoring of dependent error handling #620

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

Closed
wants to merge 4 commits into from

Conversation

zdevito
Copy link
Contributor

@zdevito zdevito commented Jul 22, 2025

Stack from ghstack (oldest at bottom):

This PR introduces a SeqError type which represents the 'root' seq that failed, clearly delinated from dependent errors in the type system.

env objects are now changed to hold Arc with the lookup functions always returning a DependentError of the seq error.

This makes it clear that our error type does not nest dependent errors, and it makes it clear where function invocation errors first get introduced.

I use this to introduce a single place that creates the Arc, reports the error to the controller, and sets it as the last error so that recordings know that error occurred.

This PR also removes partially finished work to propagate pipe creation errors to users of the pipe. We will not need pipes for much longer, and this functionality wasn't complete anyway.

Differential Revision: D78683472

NOTE FOR REVIEWERS: This PR has internal Meta-specific changes or comments, please review them on Phabricator!

This PR introduces a SeqError type which represents the 'root' seq that failed, clearly delinated from dependent errors in the type system.

env objects are now changed to hold Arc<SeqError> with the lookup functions always returning a DependentError of the seq error.

This makes it clear that our error type does not nest dependent errors, and it makes it clear where  function invocation errors first get introduced.

I use this to introduce a single place that creates the Arc<SeqError>, reports the error to the controller, and sets it as the last error so that recordings know that error occurred.

This PR also removes partially finished work to propagate pipe creation errors to users of the pipe. We will not need pipes for much longer, and this functionality wasn't complete anyway.

Differential Revision: [D78683472](https://our.internmc.facebook.com/intern/diff/D78683472/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D78683472/)!

[ghstack-poisoned]
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 22, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D78683472

…handling"

This PR introduces a SeqError type which represents the 'root' seq that failed, clearly delinated from dependent errors in the type system.

env objects are now changed to hold Arc<SeqError> with the lookup functions always returning a DependentError of the seq error.

This makes it clear that our error type does not nest dependent errors, and it makes it clear where  function invocation errors first get introduced.

I use this to introduce a single place that creates the Arc<SeqError>, reports the error to the controller, and sets it as the last error so that recordings know that error occurred.

This PR also removes partially finished work to propagate pipe creation errors to users of the pipe. We will not need pipes for much longer, and this functionality wasn't complete anyway.

Differential Revision: [D78683472](https://our.internmc.facebook.com/intern/diff/D78683472/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D78683472/)!

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D78683472

…handling"

This PR introduces a SeqError type which represents the 'root' seq that failed, clearly delinated from dependent errors in the type system.

env objects are now changed to hold Arc<SeqError> with the lookup functions always returning a DependentError of the seq error.

This makes it clear that our error type does not nest dependent errors, and it makes it clear where  function invocation errors first get introduced.

I use this to introduce a single place that creates the Arc<SeqError>, reports the error to the controller, and sets it as the last error so that recordings know that error occurred.

This PR also removes partially finished work to propagate pipe creation errors to users of the pipe. We will not need pipes for much longer, and this functionality wasn't complete anyway.

Differential Revision: [D78683472](https://our.internmc.facebook.com/intern/diff/D78683472/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D78683472/)!

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D78683472

…handling"

This PR introduces a SeqError type which represents the 'root' seq that failed, clearly delinated from dependent errors in the type system.

env objects are now changed to hold Arc<SeqError> with the lookup functions always returning a DependentError of the seq error.

This makes it clear that our error type does not nest dependent errors, and it makes it clear where  function invocation errors first get introduced.

I use this to introduce a single place that creates the Arc<SeqError>, reports the error to the controller, and sets it as the last error so that recordings know that error occurred.

This PR also removes partially finished work to propagate pipe creation errors to users of the pipe. We will not need pipes for much longer, and this functionality wasn't complete anyway.

Differential Revision: [D78683472](https://our.internmc.facebook.com/intern/diff/D78683472/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D78683472/)!

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D78683472

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 45f2585.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants