Torchvision ops registration still not fixed in C++
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 17.9k
- Forks
- 7.3k
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 13
Description
It turns out the solution I implemented for automatic ops registration in C++ is not robust enough.
If falls apart when the actual torchvision code is enclosed in a static library, which is then meant to be consumed by an executable. This indirect hop through a library exposes the pruning problem all over again. Apologies for not realizing earlier and implementing it this way :/.
In the end, I think the only two viable solutions are the ones we discussed a while ago:
- Force include all symbols through linker commands (eg.
--whole-archive) - Add, only for the C++ API, a
register_operatorsfunction that must be called in order to explicitly load the operators, and stop relying on the implicit behavior
As before my preference goes to 2 because it's simpler to implement and maintain, and it doesn't risk bloating the binary with unneeded symbols. However, option 1 is what torch already does (sadly).
Thoughts? @fmassa @datumbox
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
The issue names no files, tests, or entry points. Start by resolving whether C++ should use linker commands such as --whole-archive or an explicit register_operators call; the work is done only after one approach is agreed and the static-library case no longer loses operator registrations.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- api, computer-vision
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 20/100