Closed BenjaminHelyer closed 1 year ago
Thanks for the feedback @BenjaminHelyer !
For point 2, the flops code is designed to be used independently. The module summary adds additional metadata that may not be desirable for all users. However, we can certainly point people from the FLOPs tools to the module summary if they're looking for an expanded set of information about their modules.
Awesome, thanks so much for the explanation @ananthsub ! In that case, I think the documentation is good for both as it stands for now.
📚 The doc issue
I recently discovered this library and it looks very promising! As a newcomer, I have a couple of specific ideas about module documentation and hope they might be able to benefit the project in some way. Please excuse me if these suggestions are overly-specific: they just encapsulate a few small roadblocks that I encountered when I was first playing with the library.
Specifically, I am interested in the FLOPs and module evaluation advertised in the goals in Issue #82 . Here are my specific suggestions:
Minor fix for the docstring of FlopTensorDispatchMode. As it currently stands, the example code won't run due to a small variable naming issue. I'm making a quick PR to this effect (hope that's okay). Since flops.py was linked in Issue #82 , it was the first file I stepped into in the library, so I suspect others may have the same experience as me in the future.
Update docs with recommended usage for obtaining number of FLOPs and module summary. I wasn't sure at the beginning of the preferred way to get the number of FLOPs. The first thing I tried was the example code in FlopTensorDispatchMode, but it seems like the preferred way would be to use torcheval.tools.get_summary_table on a module. In this vein, I think we should direct users away from FlopTensorDispatchMode somehow.
Thanks for considering these suggestions; I hope to continue to be engaged in this!
Suggest a potential alternative/fix
PR will be made shortly after this issue is posted.
My feeling is that when users search 'FLOPs' in the docs or wander into flops.py, they should be directed towards torcheval.tools.get_summary_table. Add example snippet to ModuleSummary docs, possibly in torcheval.tools.get_summary_table. Also possibly add guidance in the docstring of FlopTensorDispatchMode to direct users toward ModuleSummary.