Skip to content

Conversation

@datumbox
Copy link
Contributor

@datumbox datumbox commented Sep 10, 2021

I'm converting some of the existing classes used to do computations for Detection models to nn.Modules. It's unclear why they were structured like this, could be because they were ported from DETR in the past (perhaps @fmassa can provide more info).

This PR was originally done to investigate #4386 (comment). Though it does not resolves it (might be partial solution), this conversion enables us to drop the __annotation__ declarations.

I also tried to convert the LevelMapper but it's modification is not straightforward as it modifies arguments post compilation and creates JIT issues. Worth checking this on a separate PR.



class BoxCoder(object):
class BoxCoder(nn.Module):
Copy link
Contributor Author

@datumbox datumbox Sep 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class with its encode/decode methods does not play nice with the nn.Module's forward() approach. Not sure if we should convert it but it will allow us to drop its declarations to __annotations__

def forward(self, match_quality_matrix):
return self._forward_impl(match_quality_matrix)

def _forward_impl(self, match_quality_matrix):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround for overwriting the forward on inheriting classes. We do the same on quantization.

@datumbox datumbox requested a review from fmassa September 10, 2021 16:08
@datumbox datumbox changed the title [DO_NOT_MERGE] Rewrite objects to modules Convert classes used for Detection to nn.Modules Sep 10, 2021
@datumbox datumbox marked this pull request as ready for review September 10, 2021 16:09
@datumbox
Copy link
Contributor Author

I'm going to close this PR until it's clearer to us whether we should convert Classes to Modules OR if there will be native support of Objects in JIT.

@datumbox datumbox closed this Sep 15, 2021
@datumbox datumbox deleted the obj_to_nnmodule branch September 15, 2021 11:28
@fmassa
Copy link
Member

fmassa commented Sep 15, 2021

I had a chat with Vasilis about this, and indeed the first thing to evaluate is if custom classes will be supported in the lite interpreter or not. One of the reasons for that is that converting all custom classes to nn.Module might be problematic, specially that some of them (like ImageList) can be input to the model, and thus wouldn't naturally be a nn.Module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants