With statement type #23

New Issue

pca006132 · 2021-07-20T17:26:00+08:00

pca006132 commented

2021-07-20 17:26:00 +08:00

It seems that with statement is currently used for making a parallel context. There is a discussion about deep parallel that would make all statements within a parallel block parallel to each other, but would not affect statements within a function.

Should we require expr1 in with expr1 [as expr2] to have a certain type/implement a certain interface? Like the __enter__ and __exit__ method.

It seems to me that it is possible to separate the with parallel construct from the core compiler: implement the parallel() as a context manager and implement @kernel as a decorator that would wrap the function with with sequential(). And with this strategy, only deep parallel is possible (similar to how a simulation would implement this, probably).

It seems that `with` statement is currently used for making a parallel context. There is a [discussion about *deep parallel*](https://github.com/m-labs/artiq/issues/1555) that would make all statements within a parallel block parallel to each other, but would not affect statements within a function. Should we require `expr1` in `with expr1 [as expr2]` to have a certain type/implement a certain interface? Like the `__enter__` and `__exit__` method. It seems to me that it is possible to separate the `with parallel` construct from the core compiler: implement the `parallel()` as a context manager and implement `@kernel` as a decorator that would wrap the function with `with sequential()`. And with this strategy, only deep parallel is possible (similar to how a simulation would implement this, probably).

lriesebos commented

2021-07-20 23:28:20 +08:00

It seems to me that it is possible to separate the with parallel construct from the core compiler: implement the parallel() as a context manager and implement @kernel as a decorator that would wrap the function with with sequential(). And with this strategy, only deep parallel is possible (similar to how a simulation would implement this, probably).

Just an FYI, this is what we do in our simulation. See here, where we wrap the function call in a sequential statement.

Should we require expr1 in with expr1 [as expr2] to have a certain type/implement a certain interface? Like the __enter__ and __exit__ method.

I think this is the case now. See for example this kernel-compatible context. Though currently there are a few minor limitations. The __call__ function is not supported and I am not sure if we can return a value from __enter__ for assigning to as expr2. It could be nice to have those for NAC3.

> It seems to me that it is possible to separate the with parallel construct from the core compiler: implement the parallel() as a context manager and implement @kernel as a decorator that would wrap the function with with sequential(). And with this strategy, only deep parallel is possible (similar to how a simulation would implement this, probably). Just an FYI, this is what we do in our simulation. See [here](https://gitlab.com/duke-artiq/dax/-/blob/master/dax/sim/coredevice/core.py#L88), where we wrap the function call in a `sequential` statement. > Should we require `expr1` in `with expr1 [as expr2]` to have a certain type/implement a certain interface? Like the `__enter__` and `__exit__` method. I think this is the case now. See for example [this kernel-compatible context](https://gitlab.com/duke-artiq/dax/-/blob/master/dax/modules/safety_context.py). Though currently there are a few minor limitations. The `__call__` function is not supported and I am not sure if we can return a value from `__enter__` for assigning to `as expr2`. It could be nice to have those for NAC3.

pca006132 commented

2021-07-21 10:00:11 +08:00

Just an FYI, this is what we do in our simulation. See here, where we wrap the function call in a sequential statement.

Thanks for the reference.

I think this is the case now. See for example this kernel-compatible context. Though currently there are a few minor limitations. The __call__ function is not supported and I am not sure if we can return a value from __enter__ for assigning to as expr2. It could be nice to have those for NAC3.

I don't think we need to support __call__ for this. From https://docs.python.org/3/reference/compound_stmts.html#with, the documentation mentioned:

If a target was included in the with statement, the return value from __enter__() is assigned to it.

But I think that we should be aware of the performance implication of this: Having many with context would slow down exception processing, because the __exit__() method has to be called when we exit from the suite, similar to a finally block.

Note that as we don't allow dynamic type, we would probably ignore the requirement to pass the exception object as an argument to the __exit__() function.

> Just an FYI, this is what we do in our simulation. See [here](https://gitlab.com/duke-artiq/dax/-/blob/master/dax/sim/coredevice/core.py#L88), where we wrap the function call in a `sequential` statement. Thanks for the reference. > I think this is the case now. See for example [this kernel-compatible context](https://gitlab.com/duke-artiq/dax/-/blob/master/dax/modules/safety_context.py). Though currently there are a few minor limitations. The `__call__` function is not supported and I am not sure if we can return a value from `__enter__` for assigning to `as expr2`. It could be nice to have those for NAC3. I don't think we need to support `__call__` for this. From https://docs.python.org/3/reference/compound_stmts.html#with, the documentation mentioned: > If a target was included in the with statement, the return value from `__enter__()` is assigned to it. But I think that we should be aware of the performance implication of this: Having many `with` context would slow down exception processing, because the `__exit__()` method has to be called when we exit from the suite, similar to a `finally` block. Note that as we don't allow dynamic type, we would probably ignore the requirement to pass the exception object as an argument to the `__exit__()` function.

lriesebos commented

2021-07-21 10:42:38 +08:00

I don't think we need to support __call__ for this.

That is actually true. I should elaborate a bit there. We normally do not construct the context object inside the kernel, but instead we create it earlier on the host and just pass the object to the with statement (example here). Hence, we can not use syntax such as with self.context_object(some_setting=1) as foo: configuring the context and immediately using. That would require support for the __call__ function of that object. So this would be a nice gimmic, but is not crucial.

But I think that we should be aware of the performance implication of this: Having many with context would slow down exception processing, because the __exit__() method has to be called when we exit from the suite, similar to a finally block.

Fair point. We mainly use context for safety. Slower exception processing is totally fine as long as the exception control flow is handled correctly. Currently there are some issues with that.

Note that as we don't allow dynamic type, we would probably ignore the requirement to pass the exception object as an argument to the __exit__() function.

I think that is also how it works now. As long as the __exit__() signature is the same as defined for Python, I am fine with it!

> I don't think we need to support `__call__` for this. That is actually true. I should elaborate a bit there. We normally do **not** construct the context object inside the kernel, but instead we create it earlier on the host and just pass the object to the `with` statement (example [here](https://gitlab.com/duke-artiq/dax-example/-/blob/master/repository/services/state.py#L42)). Hence, we can not use syntax such as `with self.context_object(some_setting=1) as foo`: configuring the context and immediately using. That would require support for the `__call__` function of that object. So this would be a nice gimmic, but is not crucial. > But I think that we should be aware of the performance implication of this: Having many with context would slow down exception processing, because the `__exit__()` method has to be called when we exit from the suite, similar to a `finally` block. Fair point. We mainly use context for safety. Slower exception processing is totally fine as long as the exception control flow is handled correctly. Currently there are [some issues with that](https://github.com/m-labs/artiq/issues/1478). > Note that as we don't allow dynamic type, we would probably ignore the requirement to pass the exception object as an argument to the `__exit__()` function. I think that is also how it works now. As long as the `__exit__()` signature is the same as defined for Python, I am fine with it!

sb10q commented

2021-07-21 16:53:18 +08:00

with parallel is a special construct; an ad-hoc implementation (outside the core compiler of course) sounds fine to me. We don't need special support from the compiler other than being able to plug such an ad-hoc implementation (which only needs to be notified of parallel/sequential contexts being entered and exited). Implementing the implicit top-level with sequential can be simply done in the timing management code (probably in Rust) and does not have to be an actual Python wrapper.

``with parallel`` is a special construct; an ad-hoc implementation (outside the core compiler of course) sounds fine to me. We don't need special support from the compiler other than being able to plug such an ad-hoc implementation (which only needs to be notified of parallel/sequential contexts being entered and exited). Implementing the implicit top-level ``with sequential`` can be simply done in the timing management code (probably in Rust) and does not have to be an actual Python wrapper.

pca006132 commented

2021-07-21 16:59:03 +08:00

with parallel is a special construct; an ad-hoc implementation (outside the core compiler of course) sounds fine to me. We don't need special support from the compiler other than being able to plug such an ad-hoc implementation (which only needs to be notified of parallel/sequential contexts being entered and exited).

What ad-hoc implementation are you thinking about? I think we need to design the interface if we need some sort of plugin capability for the core compiler.

Implementing the implicit top-level with sequential can be simply done in the timing management code (probably in Rust) and does not have to be an actual Python wrapper.

How exactly should we do this? Are you thinking about rewriting all calls to timing related APIs within the with parallel block into calling a parallel version of the API, and use the sequential version by default for those not in a with parallel block?

> ``with parallel`` is a special construct; an ad-hoc implementation (outside the core compiler of course) sounds fine to me. We don't need special support from the compiler other than being able to plug such an ad-hoc implementation (which only needs to be notified of parallel/sequential contexts being entered and exited). What ad-hoc implementation are you thinking about? I think we need to design the interface if we need some sort of plugin capability for the core compiler. > Implementing the implicit top-level ``with sequential`` can be simply done in the timing management code (probably in Rust) and does not have to be an actual Python wrapper. How exactly should we do this? Are you thinking about rewriting all calls to timing related APIs within the `with parallel` block into calling a parallel version of the API, and use the sequential version by default for those not in a `with parallel` block?

sb10q commented

2021-07-21 17:04:37 +08:00

What ad-hoc implementation are you thinking about?

See https://github.com/m-labs/artiq/blob/master/artiq/sim/time.py
The compiler simply needs to provide a means to call enter_sequential/enter_parallel/exit.
The stack of SequentialTimeContext/ParallelTimeContext could be in a statically allocated buffer, with a predefined maximum number of nested contexts that can be entered.

By the way, deep parallel semantics will cause the delay function to be slower than in the current design.

> What ad-hoc implementation are you thinking about? See https://github.com/m-labs/artiq/blob/master/artiq/sim/time.py The compiler simply needs to provide a means to call enter_sequential/enter_parallel/exit. The stack of SequentialTimeContext/ParallelTimeContext could be in a statically allocated buffer, with a predefined maximum number of nested contexts that can be entered. By the way, deep parallel semantics will cause the delay function to be slower than in the current design.

sb10q commented

2021-07-21 17:07:04 +08:00

By the way, deep parallel semantics will cause the delay function to be slower than in the current design.

...unless we move some of the implementation to gateware, which can be done for softcore systems (on Zynq it would actually make things slower due to the PS/PL latency).

> By the way, deep parallel semantics will cause the delay function to be slower than in the current design. ...unless we move some of the implementation to gateware, which can be done for softcore systems (on Zynq it would actually make things slower due to the PS/PL latency).

sb10q commented

2021-07-21 17:12:37 +08:00

Implementing the implicit top-level with sequential can be simply done in the timing management code (probably in Rust) and does not have to be an actual Python wrapper.

How exactly should we do this?

And in the code I linked, this is done by initializing the stack accordingly.
https://github.com/m-labs/artiq/blob/master/artiq/sim/time.py#L29

> Implementing the implicit top-level with sequential can be simply done in the timing management code (probably in Rust) and does not have to be an actual Python wrapper. > How exactly should we do this? And in the code I linked, this is done by initializing the stack accordingly. https://github.com/m-labs/artiq/blob/master/artiq/sim/time.py#L29

pca006132 commented

2021-07-21 17:22:43 +08:00

See https://github.com/m-labs/artiq/blob/master/artiq/sim/time.py
The compiler simply needs to provide a means to call enter_sequential/enter_parallel/exit.
The stack of SequentialTimeContext/ParallelTimeContext could be in a statically allocated buffer, with a predefined maximum number of nested contexts that can be entered.

Three questions:

Isn't this for simulation instead of code generation?
Isn't this using python context manager? i.e. __enter__() and __exit__(). ebb67eaeee/artiq/language/core.py (L212-L245)
I think the semantic for this is different from the existing semantic (shallow parallel) and the deep parallel semantic? What if the code in a parallel region call another function that doesn't have a with sequential block? I don't have much experience with these semantics.

By the way, deep parallel semantics will cause the delay function to be slower than in the current design.

Why?

> See https://github.com/m-labs/artiq/blob/master/artiq/sim/time.py > The compiler simply needs to provide a means to call enter_sequential/enter_parallel/exit. > The stack of SequentialTimeContext/ParallelTimeContext could be in a statically allocated buffer, with a predefined maximum number of nested contexts that can be entered. Three questions: 1. Isn't this for simulation instead of code generation? 2. Isn't this using python context manager? i.e. `__enter__()` and `__exit__()`. https://github.com/m-labs/artiq/blob/ebb67eaeee8386cb03efbb543950968b3be55981/artiq/language/core.py#L212-L245 3. I think the semantic for this is different from the existing semantic (*shallow* parallel) and the deep parallel semantic? What if the code in a parallel region call another function that doesn't have a `with sequential` block? I don't have much experience with these semantics. > By the way, deep parallel semantics will cause the delay function to be slower than in the current design. Why?

pca006132 commented

2021-07-21 17:24:51 +08:00

I think for ad-hoc implementation, the easiest way to do this is to do AST rewrite in nac3embedded. No complicated interfaces is needed for the core compiler.

pca006132 commented

2021-07-21 17:28:42 +08:00

And I don't have a preference between implementing the shallow/deep parallel semantic . I mention the deep parallel semantic just because it should be possible to do without using any compiler level support.

Although personally I do think the deep parallel semantic would make more sense...

And I don't have a preference between implementing the shallow/deep parallel semantic . I mention the deep parallel semantic just because it should be possible to do without using any compiler level support. Although personally I do think the deep parallel semantic would make more sense...

sb10q commented

2021-07-21 17:33:18 +08:00

Currently this is for simulation, and implements deep parallel. I propose to rewrite it in Rust and integrate it in nac3embedded and/or the runtime. AST rewrite is one option, a simple API for the compiler to make it emit Rust function calls on entering/leaving context managers with certain names is another.

pca006132 commented

2021-07-21 17:36:08 +08:00

a simple API for the compiler to make it emit Rust function calls on entering/leaving context managers with certain names is another.

Isn't this just how with block works in python? Just require expr in with expr to implement the __enter__(self) and __exit__(self) function, we can type-check this and do code generation for this. Is there a reason to provide a compiler API to handle this?

> a simple API for the compiler to make it emit Rust function calls on entering/leaving context managers with certain names is another. Isn't this just how `with` block works in python? Just require `expr` in `with expr` to implement the `__enter__(self)` and `__exit__(self)` function, we can type-check this and do code generation for this. Is there a reason to provide a compiler API to handle this?

sb10q commented

2021-07-21 21:05:28 +08:00

Sure, if we have sufficient support in the compiler for context managers and for global objects (like parallel and sequential would need to be) then we can use it. But a shortcut is possible.

Sure, if we have sufficient support in the compiler for context managers and for global objects (like ``parallel`` and ``sequential`` would need to be) then we can use it. But a shortcut is possible.

Sign in to join this conversation.

No Label

No Milestone

No Assignees

3 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/nac3-spec#23