[Unity] Mark result of LazyTransformParams as impure function#15697
[Unity] Mark result of LazyTransformParams as impure function#15697slyubomirsky merged 2 commits intoapache:unityfrom
Conversation
| @R.function | ||
| @R.function(pure=False) | ||
| def main_transform_params(slice_shape_expr: R.Shape(["slice_index"])): | ||
| # we expect ToNonDataflow and RemovePurityTracking to be invoked first |
There was a problem hiding this comment.
Where does LazyTransformParams fall in the phase ordering? Do we expect it to be used early or late in compilation?
There was a problem hiding this comment.
Currently, it isn't present in the lowering flow, and is explicitly called by an end-user. (Based on grep for "LazyTransformParams" across the TVM repo.) While the comment suggests that it was intended to be used in a later stage, so far this seems like just an extra requirement for an end-user to perform, before calling a function intended for an end-user's convenience (e.g. here). By outputting the correct purity annotations, we avoid this requirement.
That said, looking at it again, for correctness, any DataflowBlock in which the mutation occurs should be broken apart into a regular BindingBlock.
There was a problem hiding this comment.
Okay, if it's not specifically meant for later stages in compilation, then this change is definitely appropriate.
There was a problem hiding this comment.
What side effects would get_item have? I can't find where it's defined. Seems a little unusual for a read to be impure.
There was a problem hiding this comment.
It wouldn't be impure due to side effects, but impure due to failure to return the same result for each execution. Because the model weights are no longer expressed as input arguments, they rely on external state.
There was a problem hiding this comment.
Ah, that's interesting. I would still call it pure since it's not modifying any state (as distinct from, say, a call to a random number generator that modifies the internal state of the generator), but the dependency on external state is a potential bit of trickiness.
There was a problem hiding this comment.
Hmm. The analogy I'd see would be calls to a get_current_time() function. Calling the function doesn't modify any of the internal state of the system, but still wouldn't be a pure function due to the dependency on external state.
There was a problem hiding this comment.
Ah, that's a good point. Most definitions of purity consider a function referring to external state not to be pure, so I think you're right. I should emphasize this in the specification, since I had neglected it myself.
Mark the rest of test cases as impure. This is a follow up of #15697
2 participants
The function produced by
LazyTransformParamshas only side effects, through the"get_item"and"set_item"PackedFunc calls, and should be marked as impure.