triage artiq issues #3

New Issue

hartytp · 2021-01-19T20:20:29+08:00

hartytp commented

2021-01-19 20:20:29 +08:00

it would be really useful if someone could go over the artiq issue tracker and collect all the compiler/language related issues and sort them by type (performance, documentation/language foot-guns, feature requests) and then comment on how the new proposal makes things better/worse. This also applies to the thread on new compiler design

pca006132 commented

2021-01-20 12:36:15 +08:00

It seems to me that quite a lot of foot-guns are not documented or discussed in the issues, probably the experienced users are used to them...

I've collected some issues that seems more related to language design and less related to particular implementation problems.

Data type confusion for ARTIQ Python #656
The builtin datatypes provided by ARTIQ-Python is under-documented for tuples. The comments in the issue also suggested better error message when unsupported operation is encountered, and provide a section in the manual to list some commonly used but unsupported Python features like dict.

This is mostly documentation issue. The new proposal makes things better by 1) Listing supported features, so future documentation would be easier. 2) Allowing constant indexing for tuples, which may help to simplify some use cases for tuples (but tuple is still not iterable).
manual: document object mutability and lifetimes #717

Still documentation issues. Partially related to #1 (regarding data passing for attributes)

Attributes in classes that have both kernel and host methods are a major footgun in artiq-python IME. Improvement here would be welcome so long as it doesn't become overly restrictive. I do think we need to be careful to gather use-cases/code examples from the current artiq-python before making some of these calls however.

Another issue is that the lifetime rule is currently broken in the compiler: compiler: Audit escape analysis for assignments #1301, compiler: Function return values (?) not life-time-tracked #1497.

The current proposal (not documented yet as I'm not sure how to properly explain what the code does...) provides a simple rule for lifetime, handles static lifetime and parameters' lifetime (not proven to be correct). Not sure if the rules are too conservative.
Returning list/array from kernel #1298

Documentation issue and Feature request? Not addressed in the current proposal, as the proposal currently focuses on the broader language issues like types and lifetime, but not yet touched on kernel attribute writeback.
super() on core device #617

Feaeture request. Seems not hard to implement, but multiple inheritance might be an issue. I might have to check how python handles super().
Type inference breaks inheritance #1119

Documentation and feature request. Current artiq-python compiler only supports code reuse for inheritance, rather than polymorphism. The current proposal added explicit cast to base class by virtual (not really sure where to put the base class annotation currently, but this is just syntactic issue).
Unexpected behavior with dynamic function calls #1451

The current proposal does not support function pointer. We can probably support it later, but lifetime issues would be a pain...
llvm/compiler: FP related slowness #1007

Performance issue and design problem, stating that the current compiler use virtual calls for methods.

This proposal, although forgot to state explicitly, disallows dynamic dispatch for method calls. Dynamic dispatch is allowed when the user cast the object to its base class via virtual, and assigning function pointer to class attribute is also prohibited.
Exception object causes memory corruption #1491

Related to implementation in core device. Exception object really requires memory allocation. This proposal does not deal with this problem.
New Compiler Scoping Issue #1542

Too long to summarize here.

And I would check other things that are not labeled as compiler, and maybe give some of my thoughts about the design issue later, after lunch.

It seems to me that quite a lot of foot-guns are not documented or discussed in the issues, probably the experienced users are used to them... I've collected some issues that seems more related to language design and less related to particular implementation problems. - [Data type confusion for ARTIQ Python #656](https://github.com/m-labs/artiq/issues/656) The builtin datatypes provided by ARTIQ-Python is under-documented for tuples. The comments in the issue also suggested better error message when unsupported operation is encountered, and provide a section in the manual to list some commonly used but unsupported Python features like `dict`. This is mostly documentation issue. The new proposal makes things better by 1) Listing supported features, so future documentation would be easier. 2) Allowing constant indexing for tuples, which may help to simplify some use cases for tuples (but tuple is still not iterable). - [manual: document object mutability and lifetimes #717](https://github.com/m-labs/artiq/issues/717) Still documentation issues. Partially related to #1 (regarding data passing for attributes) > Attributes in classes that have both kernel and host methods are a major footgun in artiq-python IME. Improvement here would be welcome so long as it doesn't become overly restrictive. I do think we need to be careful to gather use-cases/code examples from the current artiq-python before making some of these calls however. Another issue is that the lifetime rule is currently broken in the compiler: [compiler: Audit escape analysis for assignments #1301](https://github.com/m-labs/artiq/issues/1301), [compiler: Function return values (?) not life-time-tracked #1497](https://github.com/m-labs/artiq/issues/1497). The current proposal (not documented yet as I'm not sure how to properly explain what the code does...) provides a simple rule for lifetime, handles static lifetime and parameters' lifetime (not proven to be correct). Not sure if the rules are too conservative. - [Returning list/array from kernel #1298](https://github.com/m-labs/artiq/issues/1298) Documentation issue and Feature request? Not addressed in the current proposal, as the proposal currently focuses on the broader language issues like types and lifetime, but not yet touched on kernel attribute writeback. - [super() on core device #617](https://github.com/m-labs/artiq/issues/617) Feaeture request. Seems not hard to implement, but multiple inheritance might be an issue. I might have to check how python handles `super()`. - [Type inference breaks inheritance #1119](https://github.com/m-labs/artiq/issues/1119) Documentation and feature request. Current artiq-python compiler only supports code reuse for inheritance, rather than polymorphism. The current proposal added explicit cast to base class by `virtual` (not really sure where to put the base class annotation currently, but this is just syntactic issue). - [Unexpected behavior with dynamic function calls #1451](https://github.com/m-labs/artiq/issues/1451) The current proposal does not support function pointer. We can probably support it later, but lifetime issues would be a pain... - [llvm/compiler: FP related slowness #1007](https://github.com/m-labs/artiq/issues/1007) Performance issue and [design problem](https://github.com/m-labs/artiq/issues/1007#issuecomment-435744207), stating that the current compiler use virtual calls for methods. This proposal, although forgot to state explicitly, disallows dynamic dispatch for method calls. Dynamic dispatch is allowed when the user cast the object to its base class via `virtual`, and assigning function pointer to class attribute is also prohibited. - [Exception object causes memory corruption #1491](https://github.com/m-labs/artiq/issues/1491) Related to implementation in core device. Exception object really requires memory allocation. This proposal does not deal with this problem. - [New Compiler Scoping Issue #1542](https://github.com/m-labs/artiq/issues/1542) Too long to summarize here. And I would check other things that are not labeled as compiler, and maybe give some of my thoughts about the design issue later, after lunch.

pca006132 commented

2021-01-20 12:42:53 +08:00

And there are tons of bugs related to compiler performance issue and type inferencing problems.

For performance issue, I think rust would deliver some speedup (constant factor probably). This would not solve the superlinear speed regarding array compilation though.

For type inferencing, the current proposal requires every (sub-)expression to be typed, and forbids type coercion, so it should be relatively simple. Another benefit of using rust is that it requires exhaustive matching, so it would be less likely to hit unimplemented cases thus less compiler internal errors.

And there are tons of bugs related to compiler performance issue and type inferencing problems. For performance issue, I think rust would deliver some speedup (constant factor probably). This would not solve the superlinear speed regarding array compilation though. For type inferencing, the current proposal requires every (sub-)expression to be typed, and forbids type coercion, so it should be relatively simple. Another benefit of using rust is that it requires exhaustive matching, so it would be less likely to hit unimplemented cases thus less compiler internal errors.

sb10q commented

2021-01-20 12:43:34 +08:00

This would not solve the superlinear speed regarding array compilation though.

But that would be solved by the simpler type system, right?

> This would not solve the superlinear speed regarding array compilation though. But that would be solved by the simpler type system, right?

pca006132 commented

2021-01-20 12:57:31 +08:00

This would not solve the superlinear speed regarding array compilation though.

But that would be solved by the simpler type system, right?

Probably, I don't see anything in the current type system that would cause array compilation to have superlinear speed. Is the current artiq-python compiler so slow as it sets up many type equations and try to solve them?

I think I also have to look at how to type check the thing via pyo3...

> > This would not solve the superlinear speed regarding array compilation though. > > But that would be solved by the simpler type system, right? Probably, I don't see anything in the current type system that would cause array compilation to have superlinear speed. Is the current artiq-python compiler so slow as it sets up many type equations and try to solve them? I think I also have to look at how to type check the thing via pyo3...

pca006132 commented

2021-01-20 14:51:03 +08:00

Other issues:

How to identify whether a class would be compiled or not? In #1119 the class is not annotated but can be used in the kernel. I think it may be better to require annotations, to differentiate them and speed up compilation (less call to python interpreter).
inhomogeneous kernel lists #519

I think supporting polymorphism would be enough for this case.

Other issues are mainly implementation issues IMO, but I think there are still other design problems not in the issue tracker that worth discussing.

Other issues: - How to identify whether a class would be compiled or not? In [#1119](https://github.com/m-labs/artiq/issues/1119) the class is not annotated but can be used in the kernel. I think it may be better to require annotations, to differentiate them and speed up compilation (less call to python interpreter). - [inhomogeneous kernel lists #519](https://github.com/m-labs/artiq/issues/519) I think supporting polymorphism would be enough for this case. Other issues are mainly implementation issues IMO, but I think there are still other design problems not in the issue tracker that worth discussing.

sb10q commented

2021-01-20 15:37:51 +08:00

How to identify whether a class would be compiled or not?

I think the implicit model used in ARTIQ currently would be OK here. Start with the first kernel (i.e. the call that resulted in the compiler being invoked), and if a reference resolves to another class, then add that class to what should be compiled. This "embedding" step can still be separated from nac3core.

What annotations are you proposing otherwise? Can you post some example code showing them?

> How to identify whether a class would be compiled or not? I think the implicit model used in ARTIQ currently would be OK here. Start with the first kernel (i.e. the call that resulted in the compiler being invoked), and if a reference resolves to another class, then add that class to what should be compiled. This "embedding" step can still be separated from nac3core. What annotations are you proposing otherwise? Can you post some example code showing them?

pca006132 commented

2021-01-20 15:45:56 +08:00

How to identify whether a class would be compiled or not?

I think the implicit model used in ARTIQ currently would be OK here. Start with the first kernel (i.e. the call that resulted in the compiler being invoked), and if a reference resolves to another class, then add that class to what should be compiled. This "embedding" step can still be separated from nac3core.

What annotations are you proposing otherwise? Can you post some example code showing them?

That works, but it would be slow, especially if we do parallel compilation later, as this would require many call to the CPython runtime for resolving the identifier, which would probably need to hold the GIL.

If there are annotations, we can get all the classes and their method signatures in one go, and work on the type checking and code generation independently for every function body, similar to how headers work.

I would propose something like a device annotation like this:

from artiq.experiment import *

@device
class classA():
    def __init__(self):
        self.property = 42

@device
class classB(classA):
    pass

class InheritanceIssue(EnvExperiment):
    """ Inheritance issue
    """
    def build(self):
        self.setattr_device("core")
        self.objects = [classA(), classB()]  # doesn't work
        # self.objects = [classB(), classB()]  # works
        
    @kernel
    def run(self):
        print(self.objects[0].property)

> > How to identify whether a class would be compiled or not? > > I think the implicit model used in ARTIQ currently would be OK here. Start with the first kernel (i.e. the call that resulted in the compiler being invoked), and if a reference resolves to another class, then add that class to what should be compiled. This "embedding" step can still be separated from nac3core. > > What annotations are you proposing otherwise? Can you post some example code showing them? That works, but it would be slow, especially if we do parallel compilation later, as this would require many call to the CPython runtime for resolving the identifier, which would probably need to hold the GIL. If there are annotations, we can get all the classes and their method signatures in one go, and work on the type checking and code generation independently for every function body, similar to how headers work. I would propose something like a `device` annotation like this: ```python from artiq.experiment import * @device class classA(): def __init__(self): self.property = 42 @device class classB(classA): pass class InheritanceIssue(EnvExperiment): """ Inheritance issue """ def build(self): self.setattr_device("core") self.objects = [classA(), classB()] # doesn't work # self.objects = [classB(), classB()] # works @kernel def run(self): print(self.objects[0].property) ```

sb10q commented

2021-01-20 16:00:03 +08:00

That works, but it would be slow, especially if we do parallel compilation later, as this would require many call to the CPython runtime for resolving the identifier, which would probably need to hold the GIL.

Yes, the GIL would need to be used.

If there are annotations, we can get all the classes and their method signatures in one go, and work on the type checking and code generation independently for every function body, similar to how headers work.

Won't there be the same problem if the class definitions are spread across several Python modules?

I would propose something like a device annotation like this:

@device is not a good name since some of the classes would be user-defined parts of an experiment and not necessarily device drivers. How about simply using the same decorator @kernel?

It cannot be derived from the presence of @kernel or @portable decorators on the methods of a class, since it is valid to have data-only classes used in kernels.

> That works, but it would be slow, especially if we do parallel compilation later, as this would require many call to the CPython runtime for resolving the identifier, which would probably need to hold the GIL. Yes, the GIL would need to be used. > If there are annotations, we can get all the classes and their method signatures in one go, and work on the type checking and code generation independently for every function body, similar to how headers work. Won't there be the same problem if the class definitions are spread across several Python modules? > I would propose something like a device annotation like this: ``@device`` is not a good name since some of the classes would be user-defined parts of an experiment and not necessarily device drivers. How about simply using the same decorator ``@kernel``? It cannot be derived from the presence of ``@kernel`` or ``@portable`` decorators on the methods of a class, since it is valid to have data-only classes used in kernels.

pca006132 commented

2021-01-20 16:11:18 +08:00

Won't there be the same problem if the class definitions are spread across several Python modules?

Initially we would just hold the GIL until we fetched all the dependencies. Without any checking and code generation, this should be fast.

Otherwise we would have to call the CPython interpreter when we are checking the function bodies, this would require us to have a way to communicate with CPython interpreter, probably with a callback in nac3core (currently implemented, but not sure if there would be some lifetime issue, in the worse case we would have to use multithread and sync_channel to resolve the problem...), and getting/releasing the lock frequently which may have some performance penalty.

@device is not a good name since some of the classes would be user-defined parts of an experiment and not necessarily device drivers. How about simply using the same decorator @kernel?

OK, I think the name is not a great deal at this stage.

It cannot be derived from the presence of @kernel or @portable decorators on the methods of a class, since it is valid to have data-only classes used in kernels.

Yes

> Won't there be the same problem if the class definitions are spread across several Python modules? Initially we would just hold the GIL until we fetched all the dependencies. Without any checking and code generation, this should be fast. Otherwise we would have to call the CPython interpreter when we are checking the function bodies, this would require us to have a way to communicate with CPython interpreter, probably with a callback in nac3core (currently implemented, but not sure if there would be some lifetime issue, in the worse case we would have to use multithread and `sync_channel` to resolve the problem...), and getting/releasing the lock frequently which may have some performance penalty. > ``@device`` is not a good name since some of the classes would be user-defined parts of an experiment and not necessarily device drivers. How about simply using the same decorator ``@kernel``? OK, I think the name is not a great deal at this stage. > It cannot be derived from the presence of ``@kernel`` or ``@portable`` decorators on the methods of a class, since it is valid to have data-only classes used in kernels. Yes

sb10q commented

2021-01-20 16:31:03 +08:00

Otherwise we would have to call the CPython interpreter when we are checking the function bodies, this would require us to have a way to communicate with CPython interpreter, probably with a callback in nac3core

Right, but what if you have a kernel that references a class that is defined in another module?

foo.py:

@kernel
class Foo:
    a: int32
    def __init___(self):
        self.a = 42

bar.py:

from foo import Foo

@kernel
class Bar:
    foo: Foo
    def __init__(self):
        self.foo = Foo()
    
    @kernel
    def quux(self):
        print(self.foo.a)

> Otherwise we would have to call the CPython interpreter when we are checking the function bodies, this would require us to have a way to communicate with CPython interpreter, probably with a callback in nac3core Right, but what if you have a kernel that references a class that is defined in another module? foo.py: ```python @kernel class Foo: a: int32 def __init___(self): self.a = 42 ``` bar.py: ```python from foo import Foo @kernel class Bar: foo: Foo def __init__(self): self.foo = Foo() @kernel def quux(self): print(self.foo.a) ```

pca006132 commented

2021-01-20 16:57:18 +08:00

Otherwise we would have to call the CPython interpreter when we are checking the function bodies, this would require us to have a way to communicate with CPython interpreter, probably with a callback in nac3core

Right, but what if you have a kernel that references a class that is defined in another module?

foo.py:
@kernel
class Foo:
    a: int32
    def __init___(self):
        self.a = 42
bar.py:
from foo import Foo

@kernel
class Bar:
    foo: Foo
    def __init__(self):
        self.foo = Foo()
    
    @kernel
    def quux(self):
        print(self.foo.a)

Currently, I plan to get all type signatures before starting to check method body and do codegen. So when we check Bar.__init__, we would already have the signature of Foo.__init__, no need to consult the CPython interpreter. And this requires us to know all the classes that our compiler is concerned with.

> > Otherwise we would have to call the CPython interpreter when we are checking the function bodies, this would require us to have a way to communicate with CPython interpreter, probably with a callback in nac3core > > Right, but what if you have a kernel that references a class that is defined in another module? > > foo.py: > ```python > @kernel > class Foo: > a: int32 > def __init___(self): > self.a = 42 > > ``` > > bar.py: > ```python > from foo import Foo > > @kernel > class Bar: > foo: Foo > def __init__(self): > self.foo = Foo() > > @kernel > def quux(self): > print(self.foo.a) > > ``` > Currently, I plan to get all type signatures before starting to check method body and do codegen. So when we check `Bar.__init__`, we would already have the signature of `Foo.__init__`, no need to consult the CPython interpreter. And this requires us to know all the classes that our compiler is concerned with.

sb10q commented

2021-01-20 16:59:23 +08:00

Note that both __init__ methods are not kernels and are executed on the host.

Note that both ``__init__`` methods are not kernels and are executed on the host.

pca006132 commented

2021-01-20 17:38:18 +08:00

Note that both __init__ methods are not kernels and are executed on the host.

We still have the signature for RPCs, although the type for each argument is any type...

> Note that both ``__init__`` methods are not kernels and are executed on the host. We still have the signature for RPCs, although the type for each argument is any type...

sb10q commented

2021-01-20 18:41:07 +08:00

But the foo object would already have been created (on the host) when the kernel Bar.quux is called.
How will the compiler find the Foo class in the other module, which is required by self.foo.a?

But the ``foo`` object would already have been created (on the host) when the kernel ``Bar.quux`` is called. How will the compiler find the ``Foo`` class in the other module, which is required by ``self.foo.a``?

lriesebos referenced this issue

2021-03-10 08:35:48 +08:00

Feature feedback from Duke #6

Sign in to join this conversation.