Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schema for JSON hardware description #1474

Closed
airwoodix opened this issue Jun 22, 2020 · 7 comments
Closed

Schema for JSON hardware description #1474

airwoodix opened this issue Jun 22, 2020 · 7 comments

Comments

@airwoodix
Copy link
Contributor

airwoodix commented Jun 22, 2020

ARTIQ Feature Request

Problem this request addresses

The schema for the JSON hardware description files (e.g. Sinara systems) grows dynamically with new boards and additions to the artiq_ddb_template tool.

When writing such files, the only way (I'm aware of) to know which options are supported is to read the source code. This is not very user friendly, even considering that generating custom configurations is an advanced usage of the ARTIQ/Sinara ecosystem.

Describe the solution you'd like

A JSON schema for the hardware description files.

Quick attempt for DIO and Urukul (no ambition to be complete)

{
    "$id": "https://example.com/sinara-system.schema.json",
    "$schema": "http://json-schema.org/draft-07/schema#",
    "description": "Sinara system definition",

    "type": "object",
    "required": ["target", "variant", "hw_rev", "base", "peripherals"],

    "properties": {
	"target": {
	    "description": "Target name",
	    "type": "string",
	    "enum": ["kasli"]
	},

	"variant": {
	    "description": "Name of the variant",
	    "type": "string"
	},

	"hw_rev": {
	    "description": "Target hardware revision",
	    "$ref": "#/definitions/version"
	},

	"base": {
	    "description": "SoC base",
	    "type": "string",
	    "enum": ["master", "satellite"]
	},

	"core_addr": {
	    "type": "string",
	    "anyOf": [
		{ "format": "hostname" },
		{ "format": "ipv4" },
		{ "format": "ipv6" }
	    ]
	},

	"rtio_frequency": {
	    "type": "number",
	    "minimum": 0
	},

	"peripherals": {
	    "type": "array",
	    "items": { "$ref": "#/definitions/peripheral" }
	}
    },

    "definitions": {
	"version": {
	    "type": "string",
	    "pattern": "^v[0-9]+(\\.[0-9]+)?$"
	},

	"eem-ports": {
	    "type": "array",
	    "minItems": 1,
	    "maxItems": 2,
	    "items": { "type": "integer" },
	    "uniqueItems": true
	},

	"eem-port": {
	    "type": "array",
	    "minItems": 1,
	    "maxItems": 1,
	    "items": { "type": "integer" },
	    "uniqueItems": true
	},

	"peripheral": {
	    "type": "object",
	    "required": ["type"],
	    "properties": {
		"type": { "type": "string" }
	    },
	    "allOf": [
		{
		    "if": { "properties": { "type": { "const": "dio" } } },
		    "then": { "$ref": "#/definitions/dio" }
		},
		{
		    "if": { "properties": { "type": { "const": "urukul" } } },
		    "then": { "$ref": "#/definitions/urukul" }
		}
	    ]
	},

	"dio": {
	    "type": "object",
	    "required": ["ports", "bank_direction_low", "bank_direction_high"],
	    "properties": {
		"type": { "const": "dio" },
		"hw_rev": { "$ref": "#/definitions/version" },
		"ports": { "$ref": "#/definitions/eem-ports" },
		"bank_direction_low": {
		    "type": "string",
		    "enum": ["input", "output"]
		},
		"bank_direction_high": {
		    "type": "string",
		    "enum": ["input", "output"]
		},
		"edge_counter": { "type": "boolean" }
	    }
	},

	"urukul": {
	    "type": "object",
	    "required": ["ports"],
	    "additionalProperties": false,
	    "properties": {
		"type": { "const": "urukul" },
		"hw_rev": { "$ref": "#/definitions/version" },
		"ports": { "$ref": "#/definitions/eem-ports" },
		"dds": {
		    "type": "string",
		    "enum": ["ad9910", "ad9912"]
		},

		"synchronization": { "type": "boolean" },

		"refclk": {
		    "type": "number",
		    "minimum": 0
		},
		"clk_sel": {
		    "type": "integer",
		    "minimum": 0,
		    "maximum": 3
		},
		"clk_div": {
		    "type": "integer",
		    "minimum": 0,
		    "maximum": 3
		},

		"pll_n": { "type": "integer" },
		"pll_vco": { "type": "integer" }
	    },

	    "if": {
		"properties": { "dds": { "const": "ad9910" } }
	    },
	    "then": {
		"properties": {
		    "pll_vco": {
			"type": "integer",
			"minimum": 0,
			"maximum": 5
		    },
		    "pll_n": {
			"type": "integer",
			"minimum": 12,
			"maximum": 127
		    }
		}
	    },
	    "else": {
		"properties": {
		    "pll_n": {
			"type": "integer",
			"minimum": 2,
			"maximum": 33
		    }
		}
	    }
	}
    }
}

Additional context

Pros:

  • documentation (also user friendly, see the title and description properties);
  • programmatic way to validate JSON hardware description files.

Cons:

  • heavy information duplication with frontend.artiq_ddb_template, gateware.targets.kasli_generic, and coredevice drivers documentation (the latter is the most problematic, a schema is essentially documentation for the former two);
  • some features are missing. In the example above, the fields pll_n and pll_vco need to be defined, then constrained for the DDS chip variants, otherwise "additionalProperties": false breaks the whole schema (see here). Rejecting additional properties is IMHO more important here.

Also:

  • the schemas for individual peripherals can be put in separate files (with dedicated "$id"s) to ease up maintenance;
  • there's in principle support for default values that would superseed the dict.get calls e.g. in artiq_ddb_template and document these defaults as well.
@sbourdeauducq
Copy link
Member

When writing such files, the only way (I'm aware of) to know which options are supported is to read the source code. This is not very user friendly,

Those programs are essentially "scratch my own itch" tools that we use in-house at M-Labs and QUARTIQ, and released as-is.

heavy information duplication with frontend.artiq_ddb_template, gateware.targets.kasli_generic

Can those read the schema? Maybe there could be schema entries that tell the machine how to handle each peripheral in the gateware and device DB.

@airwoodix
Copy link
Contributor Author

Thanks for the feedback.

Those programs are essentially "scratch my own itch" tools that we use in-house at M-Labs and QUARTIQ, and released as-is.

I understand. I feel like they should nevertheless be made more user friendly. Even if not building gateware/firmware, the JSON files are really convenient to contain the full system description and generate the DDB from it (e.g. when changing Urukul clocking).
I guess #1410 is loosely related here. I'm grateful that these scripts are released but one could argue that non-vendor modifications are just not supported. Although users would write a myriad of inconsistent scripts to address that.

Can those read the schema? Maybe there could be schema entries that tell the machine how to handle each peripheral in the gateware and device DB.

This starts to feel a lot like dataclasses or even something like pydantic and a OO-heavy design. The JSON schema could be generated from the Python (data)class definition for convenience. But this doesn't look like the general design choice of these tools.

The JSON schema validators can extract default values from the schema. What to do with the data is written in the scripts. I'm mostly afraid that the schemas get quickly out of date because they're not really needed by artiq_ddb_template or kasli_generic if they stay written the way they're now.

@drewrisinger
Copy link
Contributor

As someone who read the source code when writing a custom EEM module (see https://github.com/drewrisinger/entangler-core/blob/v1.1.1/example/entangler_gateware_example.json), I think this area could use some improvement, and a schema like this would be a good example of standardization.

However, as with much of ARTIQ, I think there's a fundamental user interest issue: many of the physicist users just want SOMETHING to work with ARTIQ & aren't interested/don't have the skills for investing to fix the pain points they encounter. And the engineers who have more of the background/inclination to fix these issues are stretched too thin to adequately address these issues (I know I am).

@airwoodix
Copy link
Contributor Author

@drewrisinger Do you have experience with JSON schemas and associated tooling? I see two problematic aspects:

  1. reading the schema is not particularly easier than reading artiq_ddb_template and kasli_generic. Are there good tools to generate a nice documentation from a schema? (e.g. jsonschema2md)
  2. the code duplication between the schema, artiq_ddb_template, and the coredevice driver is very annoying. I'd rather lean towards something like pydantic. This would go in the direction of a board support package in the linux kernel sense. Generating the schema could still be useful if there's a better documentation generator.

@drewrisinger
Copy link
Contributor

drewrisinger commented Dec 15, 2020

@airwoodix no, I don't have much experience. The main reference I have for JSON schemas is lightly skimming through https://github.com/Qiskit/qiskit-terra/tree/master/qiskit/schemas.

  1. No, no idea about nice documentation. I explicitly haven't seen anything like this in the qiskit-terra repo.
  2. What do you mean by code duplication re coredevice driver? Looking at e.g. https://github.com/m-labs/artiq/blob/master/artiq/coredevice/ad9912.py I'm not sure how you'd reduce the code for it (much) with a JSON schema.

@airwoodix
Copy link
Contributor Author

  1. I quickly tried json-schema-for-humans: the output is nice, it can in principle do the job.

  2. Most of the arguments to the coredevice driver's __init__ are given by the JSON entry (refclk, pll_vco, pll_n, etc. in Urukul/AD9910). I guess this could also be autogenerated but is surely too complex overengineering.

@astro astro mentioned this issue Dec 29, 2020
11 tasks
@sbourdeauducq
Copy link
Member

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants