Skip to content

Commit

Permalink
RE and widget updates (#1080)
Browse files Browse the repository at this point in the history
In trying to add some desired features and improvements, I hit a few
existing bugs and glitches. In the process of trying to address those I
ended up doing quite a bit of refactoring. (This PR actually removes
more code than it adds, despite adding several new features).

- Fixed issue where result name was blank if only one row was present
(i.e. the run selected just one qubit type)
- Note: There is still a bug with sometimes losing the current selection
when new results/rows are added, which can cause the row 'name' to
change due to the recent change to dynamically calculate the series/row
name. This is because the name is used as the unique key for some saved
state. I look more into this later.
- Fixed issue where multiple selections could occur on the scatter
chart.
- Fixed issue where an error was reporting in the editor for UX
components importing from the ticks module.
- Fixed some color rendering issues with JupyterLab in the browser in
dark theme.
- Made some of the DOM manipulation code more declarative and canonical,
reducing code size and complexity.
- Updated hover, selection, and tooltip display behavior to be more
consistent.
- Added keyboard up/down handler to change the selected row when the
results table is focused.
- The column selection menu now closes when you click outside of it.
- Minor adjustments to chart layout (e.g. reducing the left padding)

I think we want much of this for 1.1, so hopefully can get it in
tomorrow (Monday). I've tested in VS Code and Jupyter Notebooks. CC
@ivanbasov
  • Loading branch information
billti authored Jan 30, 2024
1 parent 8f65f2e commit 585aa36
Show file tree
Hide file tree
Showing 11 changed files with 407 additions and 467 deletions.
2 changes: 2 additions & 0 deletions npm/src/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,5 @@ export type {
} from "../lib/web/qsc_wasm.js";
export { type IStructStepResult, StepResultId } from "../lib/web/qsc_wasm.js";
export { type LanguageServiceEvent } from "./language-service/language-service.js";

export * as utils from "./utils.js";
2 changes: 2 additions & 0 deletions npm/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,5 @@ export function getLanguageServiceWorker(): ILanguageServiceWorker {

return proxy;
}

export * as utils from "./utils.js";
37 changes: 37 additions & 0 deletions npm/src/ux/ticks.ts → npm/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,40 @@ export function CreateTimeTicks(min: number, max: number): Tick[] {

return result;
}

type SeriesOfPoints = Array<{ items: Array<{ x: number; y: number }> }>;

// Given an array of object, and each of those objects has an 'items' property
// that is another array of objects with number properties x & y, return a
// tuple of [minX, maxX, minY, maxY] covering all items given
export function getMinMaxXYForItems(
data: SeriesOfPoints,
): [number, number, number, number] {
return data.reduce(
(priorSeriesResult, curr) =>
curr.items.reduce(
(priorItemResult, curr) => [
Math.min(priorItemResult[0], curr.x),
Math.max(priorItemResult[1], curr.x),
Math.min(priorItemResult[2], curr.y),
Math.max(priorItemResult[3], curr.y),
],
priorSeriesResult,
),
[Number.MAX_VALUE, Number.MIN_VALUE, Number.MAX_VALUE, Number.MIN_VALUE],
);
}

export function getRanges(data: SeriesOfPoints, rangeCoefficient: number) {
const [minX, maxX, minY, maxY] = getMinMaxXYForItems(data);

const rangeX = {
min: minX / rangeCoefficient,
max: maxX * rangeCoefficient,
};
const rangeY = {
min: minY / rangeCoefficient,
max: maxY * rangeCoefficient,
};
return { rangeX, rangeY };
}
6 changes: 3 additions & 3 deletions npm/test/basics.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import {
getLanguageService,
getLanguageServiceWorker,
getDebugServiceWorker,
utils,
} from "../dist/main.js";
import { QscEventTarget } from "../dist/compiler/events.js";
import { getAllKatas, getExerciseSources, getKata } from "../dist/katas.js";
import samples from "../dist/samples.generated.js";
import { CreateIntegerTicks, CreateTimeTicks } from "../dist/ux/ticks.js";

/** @type {import("../dist/log.js").TelemetryEvent[]} */
const telemetryEvents = [];
Expand Down Expand Up @@ -1146,7 +1146,7 @@ function getLabels(ticks) {
function runAndAssertIntegerTicks(min, max, expected) {
const message = `min: ${min}, max: ${max}`;
assert.deepStrictEqual(
getValues(CreateIntegerTicks(min, max)),
getValues(utils.CreateIntegerTicks(min, max)),
expected,
message,
);
Expand All @@ -1155,7 +1155,7 @@ function runAndAssertIntegerTicks(min, max, expected) {
function runAndAssertTimeTicks(min, max, expected) {
const message = `min: ${min}, max: ${max}`;
assert.deepStrictEqual(
getLabels(CreateTimeTicks(min, max)),
getLabels(utils.CreateTimeTicks(min, max)),
expected,
message,
);
Expand Down
3 changes: 0 additions & 3 deletions npm/ux/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export type ReData = {
tfactory: any;
errorBudget: any;
logicalCounts: any;
new: boolean;
frontierEntries: FrontierEntry[];
};

Expand All @@ -23,7 +22,6 @@ export type SingleEstimateResult = {
tfactory: any;
errorBudget: any;
logicalCounts: any;
new: boolean;
};

export type FrontierEntry = {
Expand Down Expand Up @@ -62,7 +60,6 @@ export function CreateSingleEstimateResult(
tfactory: entry.tfactory,
errorBudget: entry.errorBudget,
logicalCounts: input.logicalCounts,
new: input.new,
};
}
}
148 changes: 62 additions & 86 deletions npm/ux/estimatesOverview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,7 @@ import {
SingleEstimateResult,
} from "./data.js";
import { ResultsTable, Row } from "./resultsTable.js";
import {
Axis,
HideTooltip,
PlotItem,
ScatterChart,
ScatterSeries,
SelectPoint,
} from "./scatterChart.js";
import { Axis, PlotItem, ScatterChart, ScatterSeries } from "./scatterChart.js";

const columnNames = [
"Run name",
Expand Down Expand Up @@ -77,7 +70,6 @@ function reDataToRow(input: ReData, color: string): Row {
},
data.physicalCounts.rqops,
data.physicalCounts.physicalQubits,
data.new ? "New" : "Cached",
],
color: color,
};
Expand Down Expand Up @@ -118,6 +110,11 @@ function reDataToRowScatter(data: ReData, color: string): ScatterSeries {
}

function createRunNames(estimatesData: ReData[]): string[] {
// If there's only 1 entry, use the shared run name
if (estimatesData.length === 1) {
return [estimatesData[0].jobParams.sharedRunName];
}

const fields: string[][] = [];

estimatesData.forEach(() => {
Expand Down Expand Up @@ -178,6 +175,7 @@ export function EstimatesOverview(props: {
setEstimate: (estimate: SingleEstimateResult | null) => void;
}) {
const [selectedRow, setSelectedRow] = useState<string | null>(null);
const [selectedPoint, setSelectedPoint] = useState<[number, number]>();

const runNameRenderingError =
props.runNames != null &&
Expand All @@ -198,49 +196,40 @@ export function EstimatesOverview(props: {
});

function onPointSelected(seriesIndex: number, pointIndex: number): void {
if (seriesIndex < 0) {
// Point was deselected
onRowSelected("");
return;
}

const data = props.estimatesData[seriesIndex];
props.setEstimate(CreateSingleEstimateResult(data, pointIndex));
const rowId = props.estimatesData[seriesIndex].jobParams.runName;
setSelectedRow(rowId);
setSelectedPoint([seriesIndex, pointIndex]);
}

function onRowSelected(rowId: string, ev?: Event) {
function onRowSelected(rowId: string) {
setSelectedRow(rowId);
// On any selection, clear the "new" flag on all rows. This ensures that
// new rows do not steal focus from the user selected row.
props.estimatesData.forEach((data) => (data.new = false));

const root = findRoot(ev);
if (root) {
HideTooltip(root);
}
if (!rowId) {
props.setEstimate(null);
setSelectedPoint(undefined);
} else {
const index = props.estimatesData.findIndex(
(data) => data.jobParams.runName === rowId,
);

if (index == -1) {
props.setEstimate(null);
setSelectedPoint(undefined);
} else {
const estimateFound = props.estimatesData[index];
setSelectedPoint([index, 0]);
props.setEstimate(CreateSingleEstimateResult(estimateFound, 0));
if (root) {
SelectPoint(index, 0, root);
}
}
}
}

function findRoot(ev?: Event): Element | undefined {
return (
(ev?.currentTarget as Element | undefined)?.closest(
".qs-estimatesOverview",
) ?? undefined
);
}

const colorRenderingError =
props.colors != null &&
props.colors.length > 0 &&
Expand All @@ -253,36 +242,32 @@ export function EstimatesOverview(props: {
? props.colors
: ColorMap(props.estimatesData.length);

if (props.isSimplifiedView) {
function getResultTable() {
return (
<div className="qs-estimatesOverview">
{runNameRenderingError != "" && (
<div class="qs-estimatesOverview-error">{runNameRenderingError}</div>
<ResultsTable
columnNames={columnNames}
rows={props.estimatesData.map((dataItem, index) =>
reDataToRow(dataItem, colormap[index]),
)}
{colorRenderingError != "" && (
<div class="qs-estimatesOverview-error">{colorRenderingError}</div>
initialColumns={initialColumns}
selectedRow={selectedRow}
onRowSelected={onRowSelected}
onRowDeleted={props.onRowDeleted}
/>
);
}

function getScatterChart() {
return (
<ScatterChart
xAxis={xAxis}
yAxis={yAxis}
data={props.estimatesData.map((dataItem, index) =>
reDataToRowScatter(dataItem, colormap[index]),
)}
<ResultsTable
columnNames={columnNames}
rows={props.estimatesData.map((dataItem, index) =>
reDataToRow(dataItem, colormap[index]),
)}
initialColumns={initialColumns}
// should be able to deselect rows for making screenshots
ensureSelected={false}
onRowDeleted={props.onRowDeleted}
selectedRow={selectedRow}
onRowSelected={onRowSelected}
/>
<ScatterChart
xAxis={xAxis}
yAxis={yAxis}
data={props.estimatesData.map((dataItem, index) =>
reDataToRowScatter(dataItem, colormap[index]),
)}
onPointSelected={onPointSelected}
/>
</div>
onPointSelected={onPointSelected}
selectedPoint={selectedPoint}
/>
);
}

Expand All @@ -294,36 +279,27 @@ export function EstimatesOverview(props: {
{colorRenderingError != "" && (
<div class="qs-estimatesOverview-error">{colorRenderingError}</div>
)}
<details open>
<summary style="font-size: 1.5em; font-weight: bold; margin: 24px 8px;">
Results
</summary>
<ResultsTable
columnNames={columnNames}
rows={props.estimatesData.map((dataItem, index) =>
reDataToRow(dataItem, colormap[index]),
)}
initialColumns={initialColumns}
selectedRow={selectedRow}
onRowSelected={onRowSelected}
// should be able to deselect rows for making screenshots
ensureSelected={false}
onRowDeleted={props.onRowDeleted}
/>
</details>
<details open>
<summary style="font-size: 1.5em; font-weight: bold; margin: 24px 8px;">
Space-time diagram
</summary>
<ScatterChart
xAxis={xAxis}
yAxis={yAxis}
data={props.estimatesData.map((dataItem, index) =>
reDataToRowScatter(dataItem, colormap[index]),
)}
onPointSelected={onPointSelected}
/>
</details>
{!props.isSimplifiedView ? (
<>
<details open>
<summary style="font-size: 1.5em; font-weight: bold; margin: 24px 8px;">
Results
</summary>
{getResultTable()}
</details>
<details open>
<summary style="font-size: 1.5em; font-weight: bold; margin: 24px 8px;">
Space-time diagram
</summary>
{getScatterChart()}
</details>
</>
) : (
<>
{getResultTable()}
{getScatterChart()}
</>
)}
</div>
);
}
Loading

0 comments on commit 585aa36

Please sign in to comment.