M-Labs
/
nac3-spec
11
0
Fork 0
Code Issues 14 Pull Requests Releases Wiki Activity

Updated specification #8

Merged
pca006132 merged 5 commits from updated into master 2021-06-07 10:06:52 +08:00
Conversation 5 Commits 5 Files Changed 1 +22 -1
1 changed files with 22 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 284ab3324c - Show all commits

23
README.md
View File

@ -18,6 +18,25 @@ kernel.
Only types supported in the kernel can be referenced.
Examples:
```python
FOO = 0
@kernel
def correct() -> int:
global FOO
return FOO + 1
@kernel
def fail_without_global() -> int:
return FOO + 2
lriesebos commented 2021-04-12 04:30:45 +08:00
Outdated
Review
Copy Link

Would it still be possible to type host-only variables and class variables?

class Foo:
    a: int
    c: ClassVar[int]  # What will this mean?
    
    def __init__(self):
        self.a = 1  # Normal / kernel-only variable
        self.b: str = 'bar'  # Host-only variable, but still typed. Is this possible?
Would it still be possible to type host-only variables and class variables? ```python class Foo: a: int c: ClassVar[int] # What will this mean? def __init__(self): self.a = 1 # Normal / kernel-only variable self.b: str = 'bar' # Host-only variable, but still typed. Is this possible? ```
pca006132 commented 2021-04-12 15:26:24 +08:00
Outdated
Review
Copy Link

I did not think about that, maybe we can add some new syntax similar to ClassVar for host only variables?

I did not think about that, maybe we can add some new syntax similar to ClassVar for host only variables?
sb10q commented 2021-04-12 16:09:56 +08:00
Outdated
Review
Copy Link

Sounds a bit messy and would perhaps negate the advantages of host type annotations (e.g. third-party tools may not understand ClassVar and complain that the type annotation is incorrect).
I propose not supporting host type annotations in kernel classes; anything that is annotated goes on the device.

Sounds a bit messy and would perhaps negate the advantages of host type annotations (e.g. third-party tools may not understand ``ClassVar`` and complain that the type annotation is incorrect). I propose not supporting host type annotations in kernel classes; anything that is annotated goes on the device.
lriesebos commented 2021-04-12 23:34:25 +08:00
Outdated
Review
Copy Link

We type practically everything in our DAX library, and I think it would be good to still allow typing of host code. Maybe you like one of these two ideas:

Allow host-only variable typing in methods or by using Immutable[]

class Foo:
    a: int  # Kernel-only variable
    c: Immutable[ClassVar[int]]  # Still allows host-variable typing
    
    def __init__(self):
        self.a = 1  # Normal / kernel-only variable
        self.b: str = 'bar'  # Host-only variable typed in method

Make kernel-only variable typing special using Kernel[] and be fully compatible with regular Python typing

class Foo:
    a: Kernel[int]  # Use Kernel[] to type kernel-only variables
    c: ClassVar[int]  # Host-only typing remains vanilla
    d: str  # Also host-only
    
    def __init__(self):
        self.a = 1  # Normal / kernel-only variable
        self.b: str = 'bar'  # Host-only variable
We type practically everything in [our DAX library](https://gitlab.com/duke-artiq/dax), and I think it would be good to still allow typing of host code. Maybe you like one of these two ideas: **Allow host-only variable typing in methods or by using `Immutable[]`** ```python class Foo: a: int # Kernel-only variable c: Immutable[ClassVar[int]] # Still allows host-variable typing def __init__(self): self.a = 1 # Normal / kernel-only variable self.b: str = 'bar' # Host-only variable typed in method ``` **Make kernel-only variable typing special using `Kernel[]` and be fully compatible with regular Python typing** ```python class Foo: a: Kernel[int] # Use Kernel[] to type kernel-only variables c: ClassVar[int] # Host-only typing remains vanilla d: str # Also host-only def __init__(self): self.a = 1 # Normal / kernel-only variable self.b: str = 'bar' # Host-only variable ```
sb10q commented 2021-04-13 08:43:52 +08:00
Outdated
Review
Copy Link

Make kernel-only variable typing special using Kernel[] and be fully compatible with regular Python typing

That option would be ok, but might need better names than Kernel (and Immutable).

> Make kernel-only variable typing special using Kernel[] and be fully compatible with regular Python typing That option would be ok, but might need better names than `Kernel` (and `Immutable`).
sb10q commented 2021-04-13 08:48:22 +08:00
Outdated
Review
Copy Link

Maybe Kernel[] / KernelImmutable[] since the two annotations are similar. And we would use "kernel" terminology in nac3core.

Maybe `Kernel[]` / `KernelImmutable[]` since the two annotations are similar. And we would use "kernel" terminology in nac3core.
lriesebos commented 2021-04-13 09:44:55 +08:00
Outdated
Review
Copy Link

Yes, I think Kernel[] and KernelImmutable[] is great and I like the fact that it is explicit!

Yes, I think `Kernel[]` and `KernelImmutable[]` is great and I like the fact that it is explicit!
pca006132 commented 2021-04-13 10:13:17 +08:00
Outdated
Review
Copy Link

Yes, it is fine for me. Did you think about the syntax I mentioned in the screenshot? Using Immutable(...) instead of Immutable[].

I'm not entirely sure if we can let IDE understand Immutable[T] == T for completion and checking, but we can do that with Immutable(T). However, one caveat is that it would be harder to let users to define the type elsewhere and use it, as the information of Immutable() would be loss. Maybe we should have a look at how typing does it for other generics.

Yes, it is fine for me. Did you think about the syntax I mentioned in the screenshot? Using `Immutable(...)` instead of `Immutable[]`. I'm not entirely sure if we can let IDE understand `Immutable[T] == T` for completion and checking, but we can do that with `Immutable(T)`. However, one caveat is that it would be harder to let users to define the type elsewhere and use it, as the information of `Immutable()` would be loss. Maybe we should have a look at how typing does it for other generics.
lriesebos commented 2021-04-13 10:36:37 +08:00
Outdated
Review
Copy Link

Tbh, I do not think the IDE autocomplete feature should be leading this choice. And besides, IDE's understand that ClassVar[T] == T, so it would not surprise me if we can also make that happen for Kernel[] and KernelImmutable[]. Brackets are the syntax used for typing in Python, so I would prefer we stick with that.

To further motivate my point of view, type checkers such as mypy go totally bad when using parenthesis. See for example this error message for TArray() (ARTIQ 6):

dax/modules/rtio_benchmark.py:206: error: Invalid type comment or annotation  [valid-type]
dax/modules/rtio_benchmark.py:206: note: Suggestion: use TArray[...] instead of TArray(...)
Tbh, I do not think the IDE autocomplete feature should be leading this choice. And besides, IDE's understand that `ClassVar[T] == T`, so it would not surprise me if we can also make that happen for `Kernel[]` and `KernelImmutable[]`. Brackets are the syntax used for typing in Python, so I would prefer we stick with that. To further motivate my point of view, type checkers such as mypy go totally bad when using parenthesis. See for example this error message for `TArray()` (ARTIQ 6): ``` dax/modules/rtio_benchmark.py:206: error: Invalid type comment or annotation [valid-type] dax/modules/rtio_benchmark.py:206: note: Suggestion: use TArray[...] instead of TArray(...) ```
lriesebos commented 2021-06-07 19:33:03 +08:00
Outdated
Review
Copy Link

@pca006132 @sb10q did we now decide on the Kernel[] and KernelImmutable[] annotations? It is not contained in this merge request.

@pca006132 @sb10q did we now decide on the `Kernel[]` and `KernelImmutable[]` annotations? It is not contained in this merge request.
pca006132 commented 2021-06-08 09:46:02 +08:00
Outdated
Review
Copy Link

Sorry I forgot this one, this is two months ago :(.

I think Kernel is used for kernel only attributes, and KernelImmutable is for kernel invariants right? Can KernelImmutable be changed by the host? I think it is OK, or we can open another issue If we want further discussion about this.

Sorry I forgot this one, this is two months ago :(. I think `Kernel` is used for kernel only attributes, and `KernelImmutable` is for kernel invariants right? Can `KernelImmutable` be changed by the host? I think it is OK, or we can open another issue If we want further discussion about this.
👍 1
sb10q commented 2021-06-08 11:41:08 +08:00
Outdated
Review
Copy Link

As per the discussion in #5, an attribute with KernelImmutable can be modified by the host, but only when no kernel is running. In particular, it cannot be modified inside a RPC call, and this would be enforced in the Python interpreter via the @kernel decorator on the class.

As per the discussion in https://git.m-labs.hk/M-Labs/nac3-spec/issues/5, an attribute with ``KernelImmutable`` can be modified by the host, but only when no kernel is running. In particular, it cannot be modified inside a RPC call, and this would be enforced in the Python interpreter via the ``@kernel`` decorator on the class.
👍 1
@kernel
pca006132 marked this conversation as resolved Outdated
lriesebos commented 2021-04-12 04:22:57 +08:00
Outdated
Review
Copy Link

Should we use brackets instead, just like used for all other typing? KernelInvariant[T]

Should we use brackets instead, just like used for all other typing? `KernelInvariant[T]`
sb10q commented 2021-04-12 16:23:34 +08:00
Outdated
Review
Copy Link

I think so.

I think so.
pca006132 commented 2021-04-12 18:08:13 +08:00
Outdated
Review
Copy Link

Yes, I would fix this later.

Yes, I would fix this later.
def fail_write() -> None:
FOO += 1
```
## Class and Functions
* Instance variables must be annotated: (Issue #1)
```python
@ -31,7 +50,7 @@ Only types supported in the kernel can be referenced.
* Three types of instance variables: (Issue #5)
* Host only variables: Do not add type annotation for it in the class.
* Kernel Invariants: Immutable in the kernel and in the host while the kernel
is executing. Type: `KernelInvariant(T)`. The types must be immutable.
is executing. Type: `KernelInvariant[T]`. The types must be immutable.
(use tuple instead of list in the host, but the type annotation should still
be list?)
* Normal Variables: The host can only assign to them in the `__init__`
@ -47,6 +66,8 @@ Only types supported in the kernel can be referenced.
* Function pointers are supported, and lambda expression is not supported
currently. (maybe support lambda after implementing type inference?)
Its type is denoted by the typing library, e.g. `Call[[int32, int32], int32]`.
## Built-in Types
* Primitive types include:
* `bool`