Repository navigation
feat(ecvit): add multi-channel input support - #951
Open
lorinczszabolcs wants to merge 8 commits into
Open
lorinczszabolcs wants to merge 8 commits into
lorinczszabolcs wants to merge 8 commits into
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
2 of 8 tasks
Contributor
|
/review |
liopeer
changed the title
Make ECViTModelWrapper implement MultiScaleFeatureCNN and EdgeCrafterPackage a MultiScaleFeaturePackage, exposing the existing (P3, P4, P5) ECViT pyramid (strides 8, 16, 32) through forward_multiscale_features. Register the four ECViT linear semantic segmentation configs and add unit tests. Part of lightly-ai#928 and lightly-ai#925.
Build the ECViT patch embed for num_input_channels and adapt pretrained RGB weights to it on load through a load_state_dict pre-hook, following the DINOv2/DINOv3/TIMM packages. Extend the shared input-channel helper with a conv-pyramid variant, drop the 3-channel restriction in EdgeCrafterPackage, update the docs, and add unit tests. Part of lightly-ai#928.
lorinczszabolcs
force-pushed
the
add-multichannel-support-edgecrafter
branch
from
September 16, 2026 09:24
d9603ff to
a11eb81
Compare
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…alized levels Addresses review findings on lightly-ai#950/lightly-ai#951: - ltdetr_object_detection: drop the edgecrafter special case that pinned _expected_input_channels to 3 while get_model was built for len(image_normalize["mean"]). A 4-channel image_normalize built a 4-channel stem but declared 3 in the preprocessor and the ONNX input spec. - dinov3_ltdetr: forward num_input_channels to EDGE_CRAFTER_PACKAGE.get_model and drop the now-false comment claiming ECViT has no multi-channel support. - ecvit_package: merge num_input_channels into model_args instead of passing it alongside **model_args, which raised TypeError when a caller set it in backbone_args. Matches DINOv3Package.get_model. - ecvit: TODO on forward_multiscale_features recording that levels are not uniformly normalized when the preset proj_dim is None. - docs: drop the edgecrafter note contradicting the new multi-channel support. - tests: narrow patch_embed/conv types so mypy passes; reflow CHANGELOG.
test_registry_has_config_for_every_backbone required an explicit config for every backbone, but 27 of 39 configs only restate backbone_name, which the Fallback path already derives. Only the 12 DINOv2 configs carry real content (freeze_mask_token=True, drop_path_rate=0.0), and Fallback keeps backbone_name="" so _check_freeze_mask_token never fires for it. Narrow the test to DINOv2 backbones and drop the four bare ECViT configs that existed only to satisfy the wider check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #928. Stacked on #950.
What has changed and why?
This is the second item of #928: multi-channel image support for the
edgecrafter(ECViT) package, following the DINOv2, DINOv3, and TIMM packages.EdgeCrafterPackagenow acceptsnum_input_channelsand builds the ECViT patch embed for that many channels. It no longer restricts the input to 3 channels.load_state_dictpre-hook. I extracted the channel surgery in_model_helpersinto one shared function so both the existing patch-embed hook and the new conv-pyramid hook use it. The existing hook's behavior is unchanged and the DINOv2/DINOv3 tests still pass.Note on the stack: this PR is stacked on #950 (the first item of #928). GitHub does not let a fork pull request use another fork branch as its base. So this PR is opened against
mainand shows both commits until #950 merges. Please review only the multi-channel commit (d9603ff5). After #950 is merged I will rebase this branch ontomainso it contains only the multi-channel commit.How has it been tested?
New unit tests in
tests/_models/ecvit/test_ecvit.pycover multi-channel forward passes (1, 4, and 6 channels), the input-conv build, and the weight adaptation on load (repeat and slice). New tests intests/_models/test_model_helpers.pycover the shared adapter and both hooks. Run them with:Did you update CHANGELOG.md?
Did you update the documentation?