Update API usage log calls in ops

[kazhang]

As we discuss in API usage logging policy(#5052), torchvision.ops needs to be updated to include calls in all class constructors. Currently there are calls on all methods.
For example, logging is triggered for roi_align, but we also need log call for RoIAlign.

[datumbox]

@kazhang Thanks for raising issues. If you add more info here to make the ticket self-explanatory without requiring reading other issues and potentially provide an example of how to fix RoiAlign in code, we will be able to tag it as "good first issue" and ask for the help of the community. Usually it takes a bit more effort to write detailed descriptions so that new contributors with no context on Vision can help.

Also tagging a bunch of regulars in case they have the bandwidth to help: @frgfm @oke-aditya @zhiqwang @xiaohu2015 :)

[frgfm]

Happy to help 👍
But I would need some clarification if possible:

• in the example of RoIAlign, considering it calls roi_align (cf.

  vision/torchvision/ops/roi_align.py

  Line 84 in 4cacf5a

  return roi_align(input, rois, self.output_size, self.spatial_scale, self.sampling_ratio, self.aligned)

  ), do we want to only add an additional log call when RoIAlign.forward is used?
• or we want to disable the underlying call from the functional API, and only have a log call for the module API?

[xiaohu2015]

@frgfm I think he want to include logging calls in all class constructors for torchvision.ops. of cource, you have to remove the underlying call in the functional API.

[datumbox]

Thanks a lot for your support :)

The original idea was why include a call on RoIAlign if a call exists on roi_align? This is a fair point but it leads to a complicated policy where one needs to check if a functional method exists before adding the call on the constructor. So we propose to simplify this by adding always calls in both the Class and the functional ops. Note that not all ops have both Class and functional parts so this way we cover everything.

Before picking this up, note that @kazhang has an unmerged PR at #5072 which slightly changes the log call. So maybe wait for a few hours until this is merged before you open PRs. He is on a different timezone, so he hasn't seen yet the messages.

[xiaohu2015]

I am sorry that I don't read the discussion #5052.

On torchvision.ops, the calls should be added both on the main class of the operator (eg StochasticDepth) and on its functional equivalent (eg stochastic_depth) if available. Note that this is not the situation as of now and this needs to be updated on the ops submodule.

[frgfm]

Before picking this up, note that @kazhang has an unmerged PR at #5072 which slightly changes the log call. So maybe wait for a few hours until this is merged before you open PRs. He is on a different timezone, so he hasn't seen yet the messages.

Sounds good, I'll take a look at it once the PR is merged!

[datumbox]

@frgfm FYI it's now merged. :)

[frgfm]

Thanks, I'm checking right now @datumbox!
I have one last question: for classes, do we want to place the log call in the constructor, or forward method? I was about to go for the second option but I saw this

vision/torchvision/models/alexnet.py

Line 21 in d716c42

_log_api_usage_once("models", self.__class__.__name__)

So I'm not entirely sure 😅

[datumbox]

@frgfm For classes the intention is to put it on the constructor not the forward method. Nevertheless, there is some additional on going discussion at #5052. Let us get back to you once we finalize this to minimize changes. Thanks for the help! :)

[kazhang]

Logging for ops constructor has been added in #5095.