Rename intermediate values to _val and _const | feat(torchlib)

[justinchuby]

Stack from ghstack (oldest at bottom):

• Implement aten::index | feat(torchlib) (#862) #883
• Simplify model error message in test | test(torchlib) #882
• -> Rename intermediate values to _val and _const | feat(torchlib) #881

The torchscript ONNX graph generator creates numeric value names by default (0, 1). These are not legal ONNX tensor names, since ONNX requires the names to be valid C variable names. This change updates the names by prepending a prefix _val_ or _const_ to make them valid ONNX names. It also improves readability by making the names less likely to be confused with shape values.

I decided to use the _ prefix to reduce the chance of name collision with FX names.

After:

<
   ir_version: 8,
   opset_import: ["" : 18],
   producer_name: "pytorch",
   producer_version: "2.1.0"
>
torch_jit (float[5,5,5,5] input_0, int64[2] input_1_3) => (float[5,5,5,2] _val_10) {
   _val_2 = Transpose <perm = [0, 1, 2, 3]> (input_0)
   _val_3 = Max (input_1_3)
   _val_4 = Shape <start = 0> (_val_3)
   _val_5 = Expand (input_1_3, _val_4)
   _const_6 = Constant <value = int64 {-1}> ()
   _val_7 = Unsqueeze (_val_5, _const_6)
   _val_8 = Concat <axis = -1> (_val_7)
   _val_9 = GatherND <batch_dims = 0> (_val_2, _val_8)
   _val_10 = Transpose <perm = [0, 1, 2, 3]> (_val_9)
}

Before:

<
   ir_version: 8,
   opset_import: ["" : 18],
   producer_name: "pytorch",
   producer_version: "2.1.0"
>
torch_jit (float[5,5,5,5] input_0, int64[2] input_1_3) => (float[5,5,5,2] 10) {
   2 = Transpose <perm = [0, 1, 2, 3]> (input_0)
   3 = Max (input_1_3)
   4 = Shape <start = 0> (3)
   5 = Expand (input_1_3, 4)
   6 = Constant <value = int64 {-1}> ()
   7 = Unsqueeze (5, 6)
   8 = Concat <axis = -1> (7)
   9 = GatherND <batch_dims = 0> (2, 8)
   10 = Transpose <perm = [0, 1, 2, 3]> (9)
}

[BowenBao]

Could you describe motivation in description?

[codecov]

Codecov Report

Merging #881 (5bde197) into main (06a0a5c) will increase coverage by 0.01%.
The diff coverage is 76.19%.

@@            Coverage Diff             @@
##             main     #881      +/-   ##
==========================================
+ Coverage   76.56%   76.57%   +0.01%     
==========================================
  Files         112      112              
  Lines       13396    13413      +17     
  Branches     1346     1349       +3     
==========================================
+ Hits        10256    10271      +15     
+ Misses       2806     2805       -1     
- Partials      334      337       +3     

Impacted Files	Coverage Δ
...nxscript/function_libs/torch_lib/graph_building.py	82.24% <76.19%> (+0.31%)	⬆️

[justinchuby]

Could you describe motivation in description?

Done

[thiagocrepaldi]

Good change.

Would the distinction between _val and _const be ambiguous for the scenario where a _val node has a constant input, which would make the node have a constant output, but with the name _val that makes it look like a non-constant?

e.g. _val_1 = Max(_const_1) mas gets a _val_1 name, but max of constant is a constant, and thus the name could actually be _const_2?

If that is the case, all nodes could be called _val_ and prevent the semantic meaning of dynamic/constant

[justinchuby]

That works too! I will update to val if no one thinks otherwise