Commit 0481db52 authored by Vacaliuc, Bogdan's avatar Vacaliuc, Bogdan
Browse files

update after 3rd prompt

parent 30337ec4
Loading
Loading
Loading
Loading
+60 −3
Original line number Diff line number Diff line
@@ -522,7 +522,34 @@ Since the scan server never writes to `Data:Y` for 2D/1D detectors, there is no
- The `WriteDataToPV` script calls `context.write(Data:XPROFILE, data, False)` with `False` (no completion wait). The scan server then separately executes `Wait(Data:XPROFILE, None)` to wait for the FittingIOC's `callbackPV()`.
- The FittingIOC's `write('Data:XPROFILE')` handler does a **second `caget()`** to fetch the real waveform data from the detector PV. This is because `context.write()` may not deliver the full waveform — it writes whatever `context.getData()` returned from the scan log, which may be a summary.

**Remaining concern:** The `context.getData(detector)` call in `WriteDataToPV` retrieves accumulated scan log data. For waveform PVs logged via `Log()`, the scan server stores the full array in its Derby database. But the FittingIOC ignores the incoming value and re-fetches via `caget()` anyway. This double-fetch is intentional but inefficient — the comment says "get vector from PV b/c scan server wont."
**Why the double-fetch is necessary (not just inefficient):**

The `WriteDataToPV` Jython script calls `context.getData(device)` to retrieve logged data. For waveform PVs, the `Log()` command correctly stores the full array (e.g., 256 elements for xprofile) as a `NumberScanSample` in the scan server's Derby database. However, the data retrieval path collapses it to a scalar:

1. `ScanDataIterator` iterates rows of logged data
2. For each sample, it calls `ScanSampleFormatter.asDouble(sample)`
3. `asDouble()` extracts **only element [0]**: `sample.getNumber(0).doubleValue()`
4. The remaining 255 elements are silently discarded

This is a design limitation in the Phoebus scan server's `ScanScriptContext.getData()` API — it was built for scalar devices and has no code path to expand multi-valued samples back into arrays. The relevant source files:

- `ScanSampleFormatter.asDouble()` — the bottleneck: `/home/controls/src/phoebus/app/scan/data/ScanSampleFormatter.java`
- `ScanScriptContext.getData()` — calls asDouble() per sample: `/home/controls/src/phoebus/app/scan/model/src/main/java/org/csstudio/scan/command/ScanScriptContext.java`
- `LogCommandImpl.execute()` — correctly stores full waveform: `/home/controls/src/phoebus/services/scan-server/src/main/java/org/csstudio/scan/server/command/LogCommandImpl.java`

So `WriteDataToPV` writes a scalar (first pixel value) to the FittingIOC PV. The FittingIOC's write handler is triggered but ignores this value and does a direct `caget()` to the original detector PV to retrieve the actual full waveform. The `WriteDataToPV` call exists solely to **trigger** the FittingIOC's write handler and provide synchronization via the subsequent `Wait(y_pv, None)`.

```
Waveform PV (256 elements)

     ├──▶ Log() → NumberScanSample([256 Numbers]) stored in Derby DB

     ├──▶ context.getData() → asDouble() extracts ONLY element[0] → scalar

     ├──▶ WriteDataToPV → context.write(Data:XPROFILE, scalar) → triggers FittingIOC

     └──▶ FittingIOC.write('Data:XPROFILE') → ignores scalar, does caget() for full 256 elements
```

### 6.4 Module-Level Flags Are Static

@@ -548,9 +575,39 @@ The scantools version of `chi_scan.py` includes `perform_fitXdist_for_chis()` (l

This function is not currently called from `FittingIOC.py`. The post-loop fit is instead handled by `trigger_fit()` which uses the `fittings` module. The `perform_fitXdist_for_chis()` function appears to be dead code left from an incomplete attempt to do custom post-loop fitting.

### 6.6 Diagnostic Image Size Mismatch
### 6.6 Diagnostic Image Sizing — Fragile Default Match

The `Diag:XY` PV is sized at 640×480 = 307,200 `uint32` elements (`FittingIOC.py:122`). The `get_img_array_from_fig_iobuf()` function renders the matplotlib figure to raw RGBA bytes, reads them as `np.uint32` (one pixel per element), and reshapes to `(w, h)`:

```python
def get_img_array_from_fig_iobuf(fig):
    with io.BytesIO() as buff:
        fig.savefig(buff, format='raw')          # RGBA, 4 bytes/pixel
        buff.seek(0)
        dd = np.frombuffer(buff.getvalue(), dtype=np.uint32)   # 1 uint32 = 1 RGBA pixel
    w, h = fig.canvas.get_width_height()
    return dd.reshape(w, h)
```

The callers use `plt.figure()` with **no explicit `figsize` or `dpi`**. Matplotlib's defaults happen to be `figsize=(6.4, 4.8)` at `dpi=100`, producing exactly 640×480 pixels — matching the PV. This is clever but fragile: any `matplotlibrc` override, environment variable (`MPLBACKEND`), or matplotlib version change could silently break the match, causing either a reshape error or garbled output.

**Recommended fix:** Make the figure dimensions explicit in all calls. Replace every:

```python
get_img_array_from_fig_iobuf(plt.figure())
```

with:

```python
get_img_array_from_fig_iobuf(plt.figure(figsize=(6.4, 4.8), dpi=100))
```

This applies to:
- `chi_scan.py` (bl4b), lines 216, 343 — in `extract_XProfile_pos()` and `extract_XY_pos()`
- `chi_scan.py` (scantools), lines 119, 164, 198 — in `extract_XProfile_pos()`, `extract_XY_pos()`, and `perform_fitXdist_for_chis()`

The `Diag:XY` PV is sized at 640×480 (307,200 elements), but `get_img_array_from_fig_iobuf()` returns whatever size the matplotlib figure canvas produces. The `reshape(w, h)` uses the canvas dimensions, which may not be 640×480 unless the figure is explicitly sized. If the figure size doesn't match, the PV write could fail or produce garbled output.
Additionally, the reshape `dd.reshape(w, h)` produces shape `(640, 480)` which is `(width, height)`. Image arrays conventionally use `(height, width)` order. Since the PV flattens the array regardless (`DiagXY.flatten()`), the ordering only matters if an OPI attempts to render it as a 2D image — in which case it should know the convention used. Adding a comment documenting this would prevent confusion.

### 6.6 Post-Loop Fit Function Mapping