Skip to content

Commit f29767e

Browse files
committed
refactor: use '.' delimiter for value and node names
Change scope separator from '/' to '.' for value names, node names, and namespace strings. The 'v_' prefix is now at the beginning of the fully qualified value name (e.g. v_layer1.attention.Add_0). Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
1 parent 3135535 commit f29767e

2 files changed

Lines changed: 19 additions & 20 deletions

File tree

onnxscript/_internal/builder.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -374,29 +374,28 @@ def _qualify_initializer_name(self, name: str) -> str:
374374
return name
375375

376376
def _qualify_value_name(self, name: str) -> str:
377-
"""Qualify a value name with the current scope using ``/`` separator.
377+
"""Qualify a value name with the current scope using ``.`` separator.
378378
379379
The name is prefixed with ``v_`` to distinguish values from parameters.
380380
"""
381381
parts = self.scope_names()
382-
qualified = f"v_{name}"
383382
if parts:
384-
return "/".join(parts) + "/" + qualified
385-
return qualified
383+
return "v_" + ".".join(parts) + "." + name
384+
return f"v_{name}"
386385

387386
def _qualify_node_name(self, name: str) -> str:
388-
"""Qualify a node name with the current scope using ``/`` separator."""
387+
"""Qualify a node name with the current scope using ``.`` separator."""
389388
parts = self.scope_names()
390389
if parts:
391-
return "/".join(parts) + "/" + name
390+
return ".".join(parts) + "." + name
392391
return name
393392

394393
def _build_namespace(self, op_type: str, domain: str = "") -> str:
395394
"""Build the namespace string for a node.
396395
397-
Format: ``scope1/scope2: domain.op_type`` or ``scope1/scope2: op_type``.
396+
Format: ``scope1.scope2: domain.op_type`` or ``scope1.scope2: op_type``.
398397
"""
399-
scope = "/".join(self.scope_names())
398+
scope = ".".join(self.scope_names())
400399
op_id = f"{domain}.{op_type}" if domain else op_type
401400
if scope:
402401
return f"{scope}: {op_id}"

onnxscript/_internal/builder_test.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -148,17 +148,17 @@ def test_value_naming_with_hierarchy(self):
148148
# Test custom names with hierarchical context
149149
op.builder.push_module("layer1")
150150
t2 = op.Mul(t1, y, _outputs=["my_mul"])
151-
self.assertEqual(t2.name, "layer1/v_my_mul")
151+
self.assertEqual(t2.name, "v_layer1.my_mul")
152152

153153
# Test nested hierarchical context with custom names
154154
op.builder.push_module("attention")
155155
t3 = op.Add(t2, x, _outputs=["my_nested_add"])
156-
self.assertEqual(t3.name, "layer1/attention/v_my_nested_add")
156+
self.assertEqual(t3.name, "v_layer1.attention.my_nested_add")
157157

158158
# Pop back and verify prefix is applied correctly
159159
op.builder.pop_module()
160160
t4 = op.Mul(t3, y, _outputs=["another_mul"])
161-
self.assertEqual(t4.name, "layer1/v_another_mul")
161+
self.assertEqual(t4.name, "v_layer1.another_mul")
162162

163163
op.builder.pop_module()
164164
t5 = op.Add(t4, x, _outputs=["final_result"])
@@ -181,13 +181,13 @@ def test_value_naming_with_ir_value_objects(self):
181181
# Test with hierarchical context
182182
op.builder.push_module("layer1")
183183
t2 = op.Mul(t1, y, _outputs=[out2])
184-
self.assertEqual(t2.name, "layer1/v_layer_output")
184+
self.assertEqual(t2.name, "v_layer1.layer_output")
185185
self.assertIs(t2, out2)
186186

187187
# Test nested hierarchical context
188188
op.builder.push_module("attention")
189189
t3 = op.Add(t2, x, _outputs=[out3])
190-
self.assertEqual(t3.name, "layer1/attention/v_nested_output")
190+
self.assertEqual(t3.name, "v_layer1.attention.nested_output")
191191
self.assertIs(t3, out3)
192192

193193
def test_default_output_naming_strategy(self):
@@ -239,20 +239,20 @@ def test_hierarchical_naming(self):
239239
# Test node and value naming with hierarchical context prefix
240240
op.builder.push_module("layer1")
241241
t3 = op.Add(t2, x)
242-
self.assertEqual(t3.name, "layer1/v_Add_2")
243-
self.assertEqual(t3.producer().name, "layer1/Add_node_2")
242+
self.assertEqual(t3.name, "v_layer1.Add_2")
243+
self.assertEqual(t3.producer().name, "layer1.Add_node_2")
244244

245245
# Test nested hierarchical context
246246
op.builder.push_module("attention")
247247
t4 = op.Mul(t3, y)
248-
self.assertEqual(t4.name, "layer1/attention/v_Mul_3")
249-
self.assertEqual(t4.producer().name, "layer1/attention/Mul_node_3")
248+
self.assertEqual(t4.name, "v_layer1.attention.Mul_3")
249+
self.assertEqual(t4.producer().name, "layer1.attention.Mul_node_3")
250250

251251
# Pop back to layer1 and verify naming continues correctly
252252
op.builder.pop_module()
253253
t5 = op.Add(t4, x)
254-
self.assertEqual(t5.name, "layer1/v_Add_4")
255-
self.assertEqual(t5.producer().name, "layer1/Add_node_4")
254+
self.assertEqual(t5.name, "v_layer1.Add_4")
255+
self.assertEqual(t5.producer().name, "layer1.Add_node_4")
256256

257257
# Pop back to root context
258258
op.builder.pop_module()
@@ -566,7 +566,7 @@ def test_node_metadata_props_namespace(self):
566566
op.builder.push_module("self_attn", "Attention")
567567
t3 = op.Add(t2, x)
568568
self.assertEqual(
569-
t3.producer().metadata_props["namespace"], "layer1/self_attn: Add"
569+
t3.producer().metadata_props["namespace"], "layer1.self_attn: Add"
570570
)
571571
op.builder.pop_module()
572572
op.builder.pop_module()

0 commit comments

Comments
 (0)