Skip to content

fix: Improve input handling for input_signature #1698

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

Merged
merged 1 commit into from
Feb 28, 2023

Conversation

gs-olive
Copy link
Collaborator

@gs-olive gs-olive commented Feb 25, 2023

Description

  • Improve error messaging for input signature arguments by verifying the nesting depth of the object is within the bounds specified in the TorchScript compiler
  • Throw user-error more quickly to avoid less-interpretable error messages later on

Relevant GraphInputs code:

if (level == 0) { // a single value like A
collection_inputs.resize(1);
collection_inputs[0].push_back(cur_input);
} else if (level == 1) { // like A in [A, A] or [(B, B), A]
collection_inputs[index].push_back(cur_input);
} else if (level == 2) { // like A in [(A, A), C]
collection_inputs[index].push_back(cur_input);
} else { // only support 2 level
LOG_ERROR(
"Input nesting depth exceeds currently supported depth (3), use 1 level: [A, B], or 2 level: [A, (B, C)]");
}
}
}

Fixes #1595

Type of change

  • Improved input handling + error messaging

Checklist:

  • [ x ] My code follows the style guidelines of this project (You can use the linters)
  • [ x ] I have performed a self-review of my own code
  • [ x ] I have commented my code, particularly in hard-to-understand areas and hacks
  • [ x ] I have made corresponding changes to the documentation
  • [ x ] I have added tests to verify my fix or my feature
  • [ x ] New and existing unit tests pass locally with my changes
  • [ x ] I have added the relevant labels to my PR in so that relevant reviewers are notified

- Improve error messaging for input signature arguments by verifying the
nesting depth of the object is within the bounds specified in the
TorchScript compiler
- Throw user-error more quickly to avoid less-interpretable error
messages later on
@gs-olive gs-olive requested a review from narendasan February 25, 2023 00:01
@gs-olive gs-olive self-assigned this Feb 25, 2023
@github-actions github-actions bot added component: api [Python] Issues re: Python API component: api [C++] Issues re: C++ API labels Feb 25, 2023
Copy link
Collaborator

@narendasan narendasan left a comment

Choose a reason for hiding this comment

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

Is 2 arbitrary or is there some sort of technical depth limit? If possible i think it would be nice to support deeper nesting like 3-4 but not a massive deal. LGTM

@gs-olive
Copy link
Collaborator Author

Is 2 arbitrary or is there some sort of technical depth limit?

The choice of maximum depth of 2 is based on the data structure used to store the inputs: std::vector<std::vector<Input>>. While this structure could be expanded via adding more nested vector structures, a more robust solution would be desirable (supporting arbitrarily complex collections).

@gs-olive gs-olive merged commit 12db9e8 into pytorch:main Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 [Bug] Encountered bug when compiling model with nested Input type using input_signature argument
3 participants