Skip to content

Conversation

@gs-olive
Copy link
Contributor

@gs-olivegs-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);
} elseif (level == 1){// like A in [A, A] or [(B, B), A]
collection_inputs[index].push_back(cur_input);
} elseif (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
Copy link
Collaborator

@narendasannarendasan 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
ContributorAuthor

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-olivegs-olive merged commit 12db9e8 into pytorch:mainFeb 28, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla signedcomponent: api [C++]Issues re: C++ APIcomponent: api [Python]Issues re: Python API

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

@gs-olive@narendasan@facebook-github-bot