Instance attributes synchronization #5
Loading…
Reference in New Issue
No description provided.
Delete Branch "%!s(<nil>)"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The Problem
Confusing Behavior
Classes can contain both kernel and host functions. However, instance attributes would only be transferred to the host after the kernel is completed, so the attributes are out of sync most of the time, and could lead to confusing behaviors.
This code produces the following output (comments added for explanation):
Even if we add
kernel_invariants
, it would only prohibit us from writing the lineself.a = 3
, but not protected from modification within a host function, as we don't (and cannot) analyze host functions.Kernel Object Construction
Currently, it is useless to construct objects in kernel code.
For example:
would yield the following error:
If we just consider how we can implement this, it looks weird. The object would be stored within the device, with no host representation. What should we do when we call an RPC method? Should we create the object in the host, and somehow identify it with an ID so the data would persist across multiple RPCs? Or should we create the object in the host, and write-back its attributes to the device after the RPC is finished? What about undeclared attributes?
Solutions
Treat as a Feature
Accept it. This is not a bug, but a feature...
More Synchronization
Synchronize attributes before and after every RPCs. There are two problems here:
Prohibit Host Functions
Prohibit host functions on kernel classes, except for the constructor.
For classes that are constructed within kernel functions, the constructor can be marked as a kernel function to avoid RPC calls. The kernel class instance in the host would be an object without attributes, the host can only call its methods.
This sounds radical, but I think most of the code is doing something similar. There are use-cases that treat attributes as kernel invariants, but I think those can be refactored into the following form:
Basically, copy its parameters to avoid accessing the inner kernel instance attributes. Duplicated logic can be refactored into standalone portable functions, without access to the class instance.
For example, after refactoring for solution 3, the
TTLClockGen
would look like this:Let me share my thoughts.
Treat it as a feature
We are aware of the limitations described above and we do not run into any problems. Most of the object attributes we use in kernels are invariant anyway with just a few exceptions. I am fine keeping it the way it is.
More sync
That would work for us but I am not sure if the performance penalty is reasonable. Especially async RPC functions are expected to have reasonable throughput. Sync RPC functions are expected to be slow, so extra synchronization is not an issue for those functions in my opinion.
Prohibit host functions
At this moment, this seems unreasonable to me. We do combine host and kernel functions in a single class often and not all situations can be conveniently solved as the given example above. See for example how we use
@portable
and kernel invariants in this code https://gitlab.com/duke-artiq/dax/-/blob/master/dax/base/scan.py#L491 or in this example where we combine kernels and non-kernel functions in one class https://gitlab.com/duke-artiq/dax/-/blob/master/dax/modules/led.py .Just to note, I work at Duke and we are daily users of ARTIQ. If there is any need to gain more insight in how we use ARTIQ, let me know!
Here's another idea:
x
is a regular Python attribute, and it is host-only. Attempting to access it from a kernel results in a compilation error.y
is a kernel-only attribute. Attempting to access it on the host outside of__init__
results in a runtime error.z
can be read from both the kernel and the host. Attempting to write it from a kernel results in a compilation error, and from the host in a runtime error. It can only be set during__init__
.The
@kernel
decorator onExample
marks the class for compilation, and also modifies the class to enforce the restrictions of (2) and (3) in the Python interpreter. (NB: Using a decorator instead of inheritance makes it easier to intercept the__init__
call)Interior mutability will create some challenges, e.g. blocking attempts to modify the contents of
Immutable(list[...])
on the host. Wrapping the list type might make things slower and/or cause compatibility problems.We could perhaps forbid
Immutable(list[...])
and only allowImmutable(tuple[...])
instead (and of course, it would have to recursively typecheck, and the tuple could not contain lists).And for
Immutable(ndarray(...))
, Numpy supports read-only arrays:https://stackoverflow.com/questions/5541324/immutable-numpy-array
Not perfect but should be ok in practice.
The latest proposal definitely sounds more reasonable, though it still blocks one of our important use cases. We often modify/configure kernel invariant variables before entering a kernel. This allows us to dynamically set up data or even functions that will be compiled into a kernel at runtime.
Maybe it could be an option to give more freedom to functions that are decorated as
@host_only
, for example that host only functions are able to set "immutable" variables. That would still resolve the potentially confusing behavior regarding attribute synchronization in RPC functions but allows us to still modify variables used in kernels in functions decorated as host only.So for example:
However, if the host function is called while the kernel is executing, e.g. via RPC, the invariant would be modified without the kernel noticing it.
@pca006132 that is a true point I did oversee. Would it be reasonable to let the
@host_only
decorator confirm at runtime if the core is running a kernel? Keeping a flag in the core device driver should not be hard. Functionality wise, I guess it is not too far away from the initial proposal of @sb10q where writing to anImmutable()
variable from the host result in aRuntimeError
.And I would also like to note again that at least for Duke, the current behavior is totally fine.
We can disallow RPC of host functions unless they are explicity decorated
@rpc
, and disallow modifyingImmutable
attributes in@rpc
methods.I like the idea of explicitly marking each
@rpc
function, making all undecorated functions "host only" by default.Yes, actually after thinking for a bit, it seems that the user can modify the instance variables without calling host methods, just RPC to the host, call a public function or something which access the object and modify its field.
I think maybe we should explicitly mark kernel and RPC functions, let the remaining functions to be host only, and warn the user about modifying instance variables in the host while the kernel is executing...
For Python, I think it would be hard to enforce mutability constraints, especially when we want the instance variables to be mutable when the kernel is not executed, but immutable when the kernel is executed. Maybe check the modification in RPC functions and warn them, but we can't really do much better than this.
I agree that it will be interesting to enforce mutability constraints on the host. Probably with the
@kernel
class decorator and overriding__setattr__()
and__getattr__()
I guess.Let me maybe summarize the instance variable types and see where we are.
__init__()
function.KernelInvariant
orImmutable
) and also special semantics. no restrictions on host access, but invariant in the kernels, and therefore there is no synchronization during or after kernel execution.So I guess most concerns are around instance variables that can be modified in kernels and can be accessed from the host. If we eliminate 4 and 5, then only 3 needs some additional documentation.
Yes. For 1 and 3, it would be simple to enforce in the kernel.
If we allow for more complex objects, we may also have to document case 2, as the host may be able to indirectly access the instance variable if it is an object.
That's not much of an issue actually, in the worst case we have a global boolean variable that says if a kernel is currently executing (easy to do in the
@kernel
decorator implementation) and the code that blocks mutability simply stops throwing errors when a kernel isn't executing.I could write some PoC code for this.
That's the "interior mutability" problem I mentioned. But, those objects would also have a
@kernel
decorator in order to be visible at all in the kernels, so we can hack them.Thanks for the PoC, but I think we have to recursively apply this to all its fields, and possibly consider the case for lists as well. Otherwise if we modify
MutDemo
it would not throw an error:But yeah, this is possible.
I suggested above to disallow
Immutable(list[...])
and only allowImmutable(tuple(...))
.Then the other objects are user-defined objects, and decorated by
@kernel
and therefore hackable.Kernel invariant / immutable lists is actually something that is used for iterating over items in kernels. Often with a lists of primitives, objects (e.g. devices), or functions. These could be converted to numpy arrays if those are still allowed to be kernel invariant / immutable. See example below.
For user-defined objects marked kernel invariant / immutable, I would expect the reference to the object to be immutable in kernels, but attributes of the object itself do not have to be immutable as this will be defined in the class definition of that object, which is also decorated with
@kernel
. This is also the current behavior with kernel invariant user-defined objects.On a side note, typing is normally done with
[]
instead of()
. I assume that is also the syntax expected for typing any ARTIQ related stuff.You can iterate over a tuple.
e.g.
In my experience, this is a major footgun in the current artiq-python language. Every new user hits attriubte synchronisation bugs. Clear documentation with examples (and an explanation of why the current behaviour is the way it is) would go a long way to resolving this, but I am also interested in exploring ways we can do better.
NB the current behaviour is made worse by the various bugs around returning lists from RPCs.
If I've understood this correctly, it feels potentially problematic (e.g. do RPCs for classes which store a lot of data internally now become really slow?)
@hartytp please read the rest of the discussion. Are there any issues with my proposal (and subsequent refinements): #5 (comment)