Can't extend EnvExperiment

[mfe5003]

Bug Report

One-Line Summary

I would like make a derived class of EnvExperiment that multiple actual experiment classes derive from, but this shows up as a second experiment in the get_experiment method.

Issue Details

In get_experiment the is_experiment check has exceptions for EnvExperiment hard coded into it, so when I make a different named derived class it doesn't exclude it and shows up as multiple experiments.

Steps to Reproduce

create a new file led.py:

from artiq.experiment import *

class CustomExperiment(EnvExperiment):
    pass

class LED(CustomExperiment):
    def build(self):
        self.setattr_device("core")
        self.setattr_device("led0")

    @kernel
    def run(self):
        self.core.reset()
        self.led0.on()

try to run:

$ artiq_run led.py

Expected Behavior

The get_experiment call should return the top-level experiment.

Actual (undesired) Behavior

(artiq-dev) ebert@Waddles:~/projects/artiq-dev/artiq/artiq/examples/kasli_tester
$ artiq_run led.py
Traceback (most recent call last):
  File "/home/ebert/miniconda3/envs/artiq-dev/bin/artiq_run", line 11, in <module>
    load_entry_point('artiq', 'console_scripts', 'artiq_run')()
  File "/mnt/c/Users/ebert/Documents/projects/artiq-dev/artiq/artiq/frontend/artiq_run.py", line 219, in main
    return run(with_file=True)
  File "/mnt/c/Users/ebert/Documents/projects/artiq-dev/artiq/artiq/frontend/artiq_run.py", line 205, in run
    raise exn
  File "/mnt/c/Users/ebert/Documents/projects/artiq-dev/artiq/artiq/frontend/artiq_run.py", line 196, in run
    exp_inst = _build_experiment(device_mgr, dataset_mgr, args)
  File "/mnt/c/Users/ebert/Documents/projects/artiq-dev/artiq/artiq/frontend/artiq_run.py", line 182, in _build_experiment
    return get_experiment(module, args.experiment)(managers)
  File "/mnt/c/Users/ebert/Documents/projects/artiq-dev/artiq/artiq/tools.py", line 105, in get_experiment
    raise ValueError("More than one experiment found in module")
ValueError: More than one experiment found in module

Your System

• Operating System: Ubuntu 18.04 and Ubuntu 16.04 WSL on Windows 10
• Conda version: 4.5.10
• ARTIQ version: 5.0.dev0+378.gbcda53ee
• Hardware involved: None

[sbourdeauducq]

What about using _CustomExperiment or __all__?

[jordens]

I usually keep the lower layers as HasEnvironment and then compose them in EnvExperiments.

[cjbe]

@mfe5003 I think this is a feature, not a bug! In my experience it is useful (in the language of your example) to have a "CustomExperiment" class that measures something and it detected as a standalone experiment, as well as a derived "LED" class that adds more functionality (e.g. runs the CustomExperiment methods and infers some adjustment to experimental parameters).

If you don't want the subclasses to be detected as experiments you can just derive from HasEnvironment (as @jordens points out) - EnvExperiment explicitly means that this is an experiment (that is, also derives from Experiment)

[mfe5003]

Thanks for the feedback everyone, breaking it down:

@sbourdeauducq changing the name to _CustomExperiment works at least in the sense that it passes this check. I haven't tested real functionality yet.

@jordens Doing the following also works, is this what you mean?

from artiq.experiment import *

class CustomExperiment(HasEnvironment):
    pass

class LED(CustomExperiment, Experiment):
    def build(self):
        self.setattr_device("core")
        self.setattr_device("led0")

    @kernel
    def run(self):
        self.core.reset()
        self.led0.on()

@cjbe Are you suggesting to add additional mixin classes to add common methods?

More generally: Is there a purpose I am missing for why the special naming scheme is required?

I am trying to add a derived class for all my experiments that during the build phase that clears the core log (like artiq_coreanalyzer) and then in the analyze phase retrieves the core log, which I would like to save to the results file. I could mixin some helper methods, but I'd have to call them explicitly which seems like unnecessary boilerplate. Is there something I am not expecting that will cause unintended issues down the line?

[jordens]

Just

class Tool(HasEnvironment):
    pass
class Exp(EnvExperiment):
    def build(self):
        self.tool = Tool(self)

OO is not always the most efficient way to solve a problem. Why not just use the scheduler? I.e. clear the buffer, submit the target experiment, wait for completion, retrieve the buffer.

[dnadlinger]

By the way, having multiple experiments in one file works in general, i.e. when used via the master. It's just artiq_{compile, run} that doesn't support it. The most consistent fix/enhancement would probably be to add an optional argument for the class name to these.

[jordens]

Those already exist

[dnadlinger]

Ah, right – I assumed they wouldn't exist, because I'm not quite sure what the issue here is, then. Either specify the Experiment class to execute, or make sure there is only one exported (__all__), or use the underscore convention.

Regarding

I am trying to add a derived class for all my experiments that during the build phase that clears the core log (like artiq_coreanalyzer) and then in the analyze phase retrieves the core log

This doesn't sound like a good plan. You can't/shouldn't access devices (such as the core log) during the build and analyze phases.

[mfe5003]

@dnadlinger

This doesn't sound like a good plan. You can't/shouldn't access devices (such as the core log) during the build and analyze phases.

I've only been running with artiq_run with simple things so far. I missed that part of the documentation, so thanks for pointing it out. Knowing that, I guess the standard use case is to setup and tear down peripherals in the run phase so as to not interfere with the pipeline.

To be explicit, the issue is that I did not understand that there is an underscore convention to do what I was trying to do. I still don't understand why the special naming scheme is necessary, but I suppose someone has a reason.

[dnadlinger]

I still don't understand why the special naming scheme is necessary

It isn't necessary. You can explicitly specify the name of the experiment class to use instead.

[sbourdeauducq]

To be explicit, the issue is that I did not understand that there is an underscore convention to do what I was trying to do.

https://hackernoon.com/understanding-the-underscore-of-python-309d1a029edc

look for _single_leading_underscore