Skip to content

[Unity][Transform] Use parameter name in BundleModelParams - #16309

Merged
Lunderberg merged 2 commits into
apache:unityfrom
Lunderberg:unity_keep_parameter_name_when_bundling
Jan 8, 2024
Merged

Lunderberg merged 2 commits into
apache:unityfrom
Lunderberg:unity_keep_parameter_name_when_bundling

Conversation

@Lunderberg

Copy link
Copy Markdown
Contributor

Prior to this commit, the BundleModelParams would replace model parameters with param_tuple[index] within expressions. These nested expressions would then be normalized, resulting in gv = param_tuple[index] or lv = param_tuple[index] variable definitions. These auto-generated gv and lv names make it quite difficult to determine which model parameter is being used.

This commit updates the BundleModelParams transform to explicitly produce the bound variable, orig_param_name = param_tuple[index], preserving human-readable names from the parameters.

Prior to this commit, the `BundleModelParams` would replace model parameters
with `param_tuple[index]` within expressions.  These nested
expressions would then be normalized, resulting in `gv =
param_tuple[index]` or `lv = param_tuple[index]` variable
definitions.  These auto-generated `gv` and `lv` names make it quite
difficult to determine which model parameter is being used.

This commit updates the `BundleModelParams` transform to explicitly
produce the bound variable, `orig_param_name = param_tuple[index]`,
preserving human-readable names from the parameters.

@slyubomirsky slyubomirsky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent quality-of-life improvement, thanks a ton.

@Lunderberg
Lunderberg merged commit fab6db2 into apache:unity Jan 8, 2024
@Lunderberg
Lunderberg deleted the unity_keep_parameter_name_when_bundling branch January 8, 2024 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants