Repository navigation
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. |
This was referenced Aug 22, 2026
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.
lorinczszabolcs
force-pushed
the
add-multiscale-features-edgecrafter
branch
from
September 16, 2026 09:24
57ab681 to
d714812
Compare
2 of 8 tasks
liopeer
approved these changes
Sep 21, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
liopeer
added a commit
to lorinczszabolcs/lightly-train
that referenced
this pull request
Sep 21, 2026
…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.
liopeer
enabled auto-merge (squash)
September 21, 2026 17:22
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.
Part of #928 and #925.
What has changed and why?
ECViTModelWrapperimplementedModelWrapperonly, so the EdgeCrafter ECViT backbone could not expose its feature pyramid through the multi-scale interface that dense prediction heads use. This adapts the existing pyramid to that interface (the first item of #928).ECViTModelWrappernow implementsMultiScaleFeatureCNN, andEdgeCrafterPackageis aMultiScaleFeaturePackage.forward_multiscale_featuresexposes the existing(P3, P4, P5)pyramid fromforward. The strides are[8, 16, 32]and all three levels share the same channel dimension.The second item of #928 (multi-channel input) is left for a separate change.
How has it been tested?
New unit tests in
tests/_models/ecvit/test_ecvit.pycover the feature dimensions and strides, output shapes, output order, invalid indices, the match withforward_featuresfor the last level, and the uniform dimensions for theproj_dim=Nonepresets. Run them with:Did you update CHANGELOG.md?
Did you update the documentation?