m-labs / artiq

A leading-edge control system for quantum information experiments
https://m-labs.hk/artiq
GNU Lesser General Public License v3.0
437 stars 201 forks source link

Get rid of kasli.py #2279

Closed shareefj closed 1 year ago

shareefj commented 1 year ago

Bug Report

One-Line Summary

Looks like there are some illegal keyword arguments being passed to all target variants.

Issue Details

Steps to Reproduce

  1. nix develop
  2. python -m artiq.gateware.targets.kasli -V master

Expected Behavior

Me has amazeballs bitfile

Actual (undesired) Behavior

$ python -m artiq.gateware.targets.kasli -V master
Traceback (most recent call last):
  File "/nix/store/c3cjxhn73xa5s8fm79w95d0879bijp04-python3-3.10.13/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/nix/store/c3cjxhn73xa5s8fm79w95d0879bijp04-python3-3.10.13/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/shareefj/git/artiq/artiq/gateware/targets/kasli.py", line 718, in <module>
    main()
  File "/home/shareefj/git/artiq/artiq/gateware/targets/kasli.py", line 713, in main
    soc = cls(**soc_kasli_argdict(args), **argdict)
  File "/home/shareefj/git/artiq/artiq/gateware/targets/kasli.py", line 648, in __init__
    MasterBase.__init__(self, hw_rev=hw_rev, **kwargs)
  File "/home/shareefj/git/artiq/artiq/gateware/targets/kasli.py", line 231, in __init__
    MiniSoC.__init__(self,
  File "/nix/store/5h4bdlcn3l332n8ch9szcb8d7si4n4d4-python3-3.10.13-env/lib/python3.10/site-packages/misoc/targets/kasli.py", line 420, in __init__
    BaseSoC.__init__(self, *args, **kwargs)
  File "/nix/store/5h4bdlcn3l332n8ch9szcb8d7si4n4d4-python3-3.10.13-env/lib/python3.10/site-packages/misoc/targets/kasli.py", line 372, in __init__
    SoCSDRAM.__init__(self, platform, cpu_reset_address=0x400000, clk_freq=clk_freq, **kwargs)
  File "/nix/store/5h4bdlcn3l332n8ch9szcb8d7si4n4d4-python3-3.10.13-env/lib/python3.10/site-packages/misoc/integration/soc_sdram.py", line 15, in __init__
    SoCCore.__init__(self, platform, clk_freq,
TypeError: SoCCore.__init__() got an unexpected keyword argument 'dds'

It looks to me like the 'dds' argument should only be passed if the 'tester' variant was selected. Modifying this to the following:

if variant == "tester":
     argdict["dds"] = args.tester_dds

soc = cls(**soc_kasli_argdict(args), **argdict)

fails in another way:

$ python -m artiq.gateware.targets.kasli -V master
DIO (EEM5) starting at RTIO channel 0x000001
Traceback (most recent call last):
  File "/nix/store/c3cjxhn73xa5s8fm79w95d0879bijp04-python3-3.10.13/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/nix/store/c3cjxhn73xa5s8fm79w95d0879bijp04-python3-3.10.13/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/shareefj/git/artiq/artiq/gateware/targets/kasli.py", line 720, in <module>
    main()
  File "/home/shareefj/git/artiq/artiq/gateware/targets/kasli.py", line 715, in main
    soc = cls(**soc_kasli_argdict(args), **argdict)
  File "/home/shareefj/git/artiq/artiq/gateware/targets/kasli.py", line 658, in __init__
    eem.Urukul.add_std(self, 0, 1, ttl_serdes_7series.Output_8X)
TypeError: Urukul.add_std() missing 1 required positional argument: 'dds_type'

Your System (omit irrelevant parts)

sbourdeauducq commented 1 year ago

I don't think anyone uses kasli.py anymore and it should be removed/trimmed/merged with kasli_generic.py.

shareefj commented 1 year ago

Your documentation says to use that flow: https://m-labs.hk/artiq/manual/developing.html

Do you want some help with a PR to clean this up? Tell me what to do and I'll have a go. What is the "correct" way to build Kasli? Using a JSON file with kasli_generic?

sbourdeauducq commented 1 year ago

For now yes but such documentation will also need to be updated again when the aforementioned removal/merge of kasli.py happens.

shareefj commented 1 year ago

What's the issue with updating documentation?

Where are there any generic JSON description files in the artiq repo? Or should I just copy one from artiq-zynq?

dnadlinger commented 1 year ago

We are still using the base classes (MasterBase, etc.) for custom gateware internally that is not exposed via the JSON interface here, so "getting rid of kasli" should only involve the default Master/Satellite and command line builder.