Enhance profile_model.py to output JSON file for benchmark #691
Conversation
|
This PR needs Approvals as follows.
Please choose reviewers and requet reviews! Click to see how to approve each reviewsYou can approve this PR by triggered comments as follows.
See all trigger commentsPlease replace [Target] to review target
|
lmnet/executor/profile_model.py
Outdated
|
|
||
| helper(prof, 0) | ||
| if node_name == "total": | ||
| node_param_dict["total_quant_size"] = res[idx][-1] |
There was a problem hiding this comment.
res[idx][-1] appear several times.
It's better to add name as normal variable.
There was a problem hiding this comment.
@iizukak
Thanks! Fixed 👍
lmnet/executor/profile_model.py
Outdated
| 'name': node_name, | ||
| 'parameters': node.total_parameters, | ||
| 'size': node.total_parameters * 32, | ||
| 'quant_size': node.total_parameters * bits, |
There was a problem hiding this comment.
Just question. Conv layer can be quantized and parameter size is bits.
Other layer type (for example, fully connected) will not be quantized.
This size calculation assume all of our parameters are Conv?
There was a problem hiding this comment.
@iizukak Thank you for your question!
No, if a layer is not a quantizable kernel, quant_size will be a same value of size.
quant_size is determined as following steps.
- check node_name is kernel or not, see is_quant_kernel
- set
bits = bit if is_quant_kernel else 32 - calc
'quant_size': node.total_parameters * bits,
This is the previous implementation, but it is difficult to understand as you mentioned.
Ummm. OK, I am planing to change these logics as follows.
node_params = node.total_parameters
node_size = node_params * 32
node_quant_size = (node_params * bit) if is_quant_kernel else None
This plan is to change quant_size is None if it cannot be quantized.
What do you think?
There was a problem hiding this comment.
This plan is to change quant_size is None if it cannot be quantized.
I tried and implement that, but it is a little complicated to use None and int in the same dictionary key's value and this implementation changes the current Markdown file format. So, I would not like to use None to follow the current format. Then, node_quant_size will be (node_params * bit) if is_quant_kernel else node_size
There was a problem hiding this comment.
@iizukak
I changed to use named values for node parameters so this becomes better to understand, I think. Please re-review this! 👍
hadusam
force-pushed
the
json_profile_model
branch
from
c55cb6d to
f709905
Compare
|
@tsawada |
There was a problem hiding this comment.
I don't fully understand but it seems this PR does multiple things that's not explained in the commit message, such as
a. changing the default --bit option to 1
b. changing print to logger
Are these changes necessary for JSON? Especially, I feel (a.) needs some wide announcement.
lmnet/executor/profile_model.py
Outdated
| return new_res | ||
| node_flops_dict = { | ||
| 'total_flops': float_res_dict["total"], | ||
| 'children': [{"name": k, "flops": float_res_dict[k]} for k in float_res_dict.keys() if k != "total"] |
There was a problem hiding this comment.
[{"name": k, "flops": v for k, v in dict.items()] ?
lmnet/executor/profile_model.py
Outdated
| 'children': node_children, | ||
| } | ||
| # Add is_quant_kernel flag to leaf node | ||
| if len(node.children) == 0: |
|
@tsawada OK, I will revert them and separate it 😄 |
|
@tsawada |
|
/ready |
|
⏳Merge job is queued... |
|
😥Status check failed. Please fix problems and send |
|
/ready |
|
⏳Merge job is queued... |
|
😥Status check failed. Please fix problems and send |
|
/ready |
|
⏳Merge job is queued... |
Motivation and Context
Current profile_model.py creates a markdown file of profile result.
We need to get a structured file of profile results for the benchmark program, so I enhanced it to create JSON file.
Description
How has this been tested?
Screenshots (if appropriate):
Types of changes
Checklist: