Add PerSAM #23652
Add PerSAM #23652
Conversation
|
The documentation is not available anymore as the PR was closed or merged. |
There was a problem hiding this comment.
Thanks for adding this. If the modification works with all checkpoints for SAM then it passes the test for which we don't require a new model. So in this instance it's fine to add a new config argument to SAM to activate persam mode and adapt the forward, which will also get rid of your warning.
| self.layer_norm1 = SamLayerNorm( | ||
| self.mask_input_channels, eps=config.layer_norm_eps, data_format="channels_first" | ||
| ) | ||
| self.layer_norm2 = SamLayerNorm( | ||
| self.mask_input_channels * 4, eps=config.layer_norm_eps, data_format="channels_first" | ||
| ) |
There was a problem hiding this comment.
@younesbelkada can you confirm if this a mistake in the SAM implementation?
There was a problem hiding this comment.
I am unsure if this is needed, @NielsRogge could you elaborate more on why this change is needed? 🙏
There was a problem hiding this comment.
Yes I had to include this fix to make input_masks work. I noticed that input_masks is not tested in tests/models/test_modeling_sam.py (only input_points, input_labels and input_boxes are).
Might be good to add a test for this
There was a problem hiding this comment.
Sounds good to me! Indeed we didn't tested input masks!
|
Ok, will close this PR in favor of modifying |
What does this PR do?
This PR adds the PerSAM model.
Question: when you do:
you get this warning:
was wondering whether we could suppress this warning. PerSAM uses the exact same weights as the original SAM model, just modifies the forward pass with 2 additional arguments. Currently the model_type is set to "persam" in
PerSamConfig.