New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nn.Parameter is ommitted (with a case) #84
Comments
Thanks, I'll investigate this issue. Notes:
|
I thought I could have a look to this issue but I'm not sure actually how to handle these I'm not completely sure to correctly understand the hooks to see if it is possible to detect when and if the parameters declared in the top level Having the impression that there is a difference between parameters in the top level
and the summary then lists the parameters (but also the fc2 ones):
Not sure how to progress but would be happy to get your feedback on the correct track to follow. As a side note, If I'm not incorrect, the test to include is not exactly the one you propose but the one below (i.e. comparing he case of using a nn.Linear for fc1 or a "raw" (W.X + b). Does it make sense ?
|
@jeremyfix First of all, I really appreciate the time you're putting in to solve this problem! Both test cases are important imo. The one I linked above demonstrates different behavior (aka missing rows in the summary table) when a module has no parameters vs when the module has at least 1 submodule (the Linear layer). The two models are not supposed to be equivalent, so the total params assertion may be unnecessary. Your example is also important because it asserts that parameters are treated the same as a module, which is what the assertion is testing. I'm not certain if this is the issue, but the issue may be related to how we treat modules in the recursive |
As a minified example, notice that
|
Hum... okay. Your last example is interesting because it shows |
I advance pretty slowly given I'm not very clear about the logic with the hooks. For clarity, let us call
For now, as far as I can tell, considering, the difference with torchinfo/torchinfo/torchinfo.py Lines 522 to 527 in d56d770
which is False for and then, it seems to me that it is the pre_hook function call (from the else case of the above test) on the Trying to find my way, I simply tried to remove the test (just for experimenting, I'm not saying this is useless) to unconditionally register pre_hooks, that allows to list
|
A follow up on my last comment. If I change the condition around lines 522-527 above with :
and also change layer_info.py:calculate_num_params() to only iterate over the module parameters (and not the submodules ones) (which I think we can discard by checking the name of the param contains a dot or not) , I get something better (not completely though):
I think basically the hook is not attached to the top level module. Maybe instead of changing the code within Also notice that for the moment, the total number of parameters is not correct. |
I can offer some insight here:
One issue to call out in your modified example is that the fc2.weight and fc2.bias are listed twice, once in the topmost layer and once in the Linear layer, when they should only be listed once. This is why we have different behavior based on the layer having submodules vs not having submodules iirc. In general, for this project I let the test cases dictate the code - anything that passes all of tests is an improvement, and anything that is missed/breaks means we are missing a test case. Beyond this, I don't have a good idea what the solution should be. |
Ok, I will have to withdraw the work on this issue. I made some progress in the output but I'm not very happy with the code I'm producing as i'm having a hard time understanding the logic behind so I prefer to stop there. My current progress in on the branch https://github.com/jeremyfix/torchinfo/tree/issue84 in which I basically added the computation of the number of parameters for the topmost level module (the one provided to the In the current version of the code, 2 tests of the test suite are failing because the
These are certainly minor things but probably required to dig into The
"Almost correct" because I did not handle in the code the computation of the MACS, therefore the This can more easily be seen from the expected output of the I'm sorry to resign but I hope that piece of work might be nevertheless of help for you. |
No worries at all, thank you so much for your help! If there's anything I can do to make the code more approachable, feel free to let me know as well. It's a tricky codebase, but I definitely want it to get better over time. |
This has been fixed in 5621cc9 and will be released in torchinfo v1.7.0. Thank you for reporting this issue! Also, thank you to @jeremyfix for the help on investigating. The above discussion was very helpful in diagnosing and fixing this issue as well as the 4+ other issues related to this problem. It required a fairly complex rewrite of the library and the code is now in a much better place. |
Thank you @TylerYep ; I'm happy this was fixed ; I'm sorry I did not respond to your request on improving the code to be more approachable; I was indeed confused by the way the recurrence was written and not clear about what was filled when ; But, I hope to dig into your code soon or later and hopefully better understand how the things are going on; |
Describe the bug
nn.Parameter
is omitted insummary
when there are other pytorch predefined layers in the networks.Details are as follows:
To Reproduce
It seems that
nn.Parameter
is not compatible with other layers (nn.Module
class).========================================================================================== Layer (type:depth-idx) Output Shape Param # ========================================================================================== FCNets -- -- ├─ReLU: 1-1 [3, 64] -- ├─Linear: 1-2 [3, 32] 2,080 ========================================================================================== Total params: 2,080 Trainable params: 2,080 Non-trainable params: 0 Total mult-adds (M): 0.01 ========================================================================================== Input size (MB): 0.00 Forward/backward pass size (MB): 0.00 Params size (MB): 0.01 Estimated Total Size (MB): 0.01 ==========================================================================================
However, if we remove
self.fc2
, the output will be fine.Pytorch version: 1.7.1 (GPU)
Torchinfo version: 1.5.3
The text was updated successfully, but these errors were encountered: