feat(field-emission): gui to display field emission data for characte…

[kvetta]

…rization purposes

[Copilot]

Pull request overview

This PR adds a new field emission application under sc_linac_physics.applications to support characterization workflows by loading field-emission measurement data from CSV/HDF5 and plotting radiation vs amplitude in a PyDM/Qt GUI.

Changes:

• Added a PyDM/Qt GUI (plus a sandbox variant) embedding Matplotlib to plot amplitude vs radiation with optional fit overlays.
• Added utilities to parse measurement metadata from a CSV and load per-cavity datasets from an HDF5 store for plotting.
• Added helper scripts for generating amplitude/radiation CSVs and for converting CSVs to HDF5; updated MANIFEST.in to include *.hdf5.

Reviewed changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 18 comments.

Show a summary per file

File	Description
src/sc_linac_physics/applications/field_emission/plot_me.py	Plotting + curve-fit overlay helper for amp vs radiation.
src/sc_linac_physics/applications/field_emission/measurements.py	CSV/HDF5 loading utilities used by the GUI.
src/sc_linac_physics/applications/field_emission/gui_sandbox.py	Sandbox GUI variant (multi-selection list widget) for field emission plotting.
src/sc_linac_physics/applications/field_emission/field_emission_gui.py	Primary field emission GUI (combo-box measurement selector) for plotting.
src/sc_linac_physics/applications/field_emission/csv_to_h5py.py	CSV→HDF5 conversion script for building the dataset store.
src/sc_linac_physics/applications/field_emission/csv_reader.py	CSV parsing utilities for measurement rows and raw metadata.
src/sc_linac_physics/applications/field_emission/amp_vs_time_from_csv.py	Script to fetch/plot amplitude vs time from CSV-described windows.
src/sc_linac_physics/applications/field_emission/amp_vs_radiation_from_csv.py	Script to fetch/align/emit amplitude vs radiation CSVs via the archiver.
src/sc_linac_physics/applications/field_emission/All FE measurements by CM.csv	Measurement log data used for GUI selection/metadata.
src/sc_linac_physics/applications/field_emission/init.py	Package marker for the new application.
MANIFEST.in	Includes *.hdf5 in sdists.

Suppressed comments (2)

src/sc_linac_physics/applications/field_emission/amp_vs_radiation_from_csv.py:127

• Call site needs to pass the readout type now that build_rad_readout_pvs() no longer relies on a global variable.

        rad_pvs = build_rad_readout_pvs(dec, rad_chans)

src/sc_linac_physics/applications/field_emission/field_emission_gui.py:365

• m = self._current_measurements[idx] will behave incorrectly when idx == -1 (last element) and will raise IndexError if _current_measurements is empty. Guard against an invalid measurement index before indexing.

    def on_plot_btn_clicked(self):
        cav = [cb.isChecked() for cb in self.cavity_cb]
        idx = self.meas_dropdown.currentIndex()
        m = self._current_measurements[idx]
        readout = self.readout_dropdown.currentText()

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.

Suppressed comments (22)

src/sc_linac_physics/applications/field_emission/field_emission_gui.py:30

• This import relies on the current working directory; use an intra-package import so the GUI is launchable from the installed package.

from plot_me import plot_amp_vs_rad

src/sc_linac_physics/applications/field_emission/gui_sandbox.py:32

• This import relies on the current working directory; use an intra-package import.

from plot_me import plot_amp_vs_rad

src/sc_linac_physics/applications/field_emission/gui_sandbox.py:389

• selected, label, and n are overwritten for each measurement in meas, so only the last selected measurement is plotted, while the subplot count is multiplied by len(meas). This makes the subplot layout inconsistent with the data being plotted.

        for m in meas:
            selected, label, n = find_dataframes(
                m["cm"], m["date"], cav, readout
            )

        # Calculate subplot rows, cols
        n = n * len(meas)
        col = math.ceil(n / 2)
        row = min(2, n)

src/sc_linac_physics/applications/field_emission/gui_sandbox.py:199

• This list widget allows multi-selection, but the plotting path below only keeps the last selected measurement (selected, label, n is overwritten in a loop). Either restrict selection to single-select, or update the plotting code to render each selected measurement distinctly.

        self.meas_list_widget.setSelectionMode(
            QAbstractItemView.ExtendedSelection
        )

src/sc_linac_physics/applications/field_emission/csv_to_h5py.py:21

• This module executes file conversion at import time and uses a hard-coded user-specific input path. Importing sc_linac_physics.applications.field_emission.csv_to_h5py in any context would immediately start filesystem I/O and fail on non-macOS/user machines.

input_path = "/Users/kvetta/Desktop/combined_data/"
input_csvs = glob.glob(os.path.join(input_path, "*.csv"))
h5_filename = "field_emission_data.hdf5"


with h5py.File(h5_filename, "w") as h5f:
    for csv_path in input_csvs:

src/sc_linac_physics/applications/field_emission/amp_vs_radiation_from_csv.py:128

• After making rad_readout_type an explicit parameter to build_rad_readout_pvs(), this call site needs to pass it in to avoid relying on a module-global.

        selected_linac = match_cryo_to_linac(cryomod)
        amp_pvs = build_amplitude_pvs(selected_linac, cryomod)
        rad_pvs = build_rad_readout_pvs(dec, rad_chans)

MANIFEST.in:10

• Adding *.hdf5 to MANIFEST.in alone may not include HDF5 files in the built wheel. The project packages data via [tool.setuptools.package-data] in pyproject.toml, which currently includes *.csv but not *.hdf5. If the GUI depends on field_emission_data.hdf5, it should be added there as well.

recursive-include src *.json
recursive-include src *.mat
recursive-include src *.csv
recursive-include src *.md
recursive-include src *.hdf5

src/sc_linac_physics/applications/field_emission/amp_vs_time_from_csv.py:114

• This hard-coded absolute output path will fail for other users/environments (and in CI) when writing the CSV output.

        aligned_data.to_csv(
            f"/Users/kvetta/sc-rad/cropped_comm/amplitudes_cm{cryo}_{stamp}.csv"
        )

src/sc_linac_physics/applications/field_emission/plot_me.py:6

• This import will fail when the package is imported/installed (it relies on the current working directory). Use an intra-package import so the module can be executed via python -m sc_linac_physics.applications.field_emission.plot_me.

from measurements import find_dataframes, get_columns

src/sc_linac_physics/applications/field_emission/measurements.py:8

• These module-level paths are relative to the current working directory. When launched via the project's CLI/launcher patterns, the CWD is not guaranteed to be this folder, so the CSV/HDF5 lookups can fail. Resolve paths relative to this module and use package-relative imports.

from csv_reader import read_from_csv, read_raw_data

input_csv = "All FE measurements by CM.csv"
h5_filename = "field_emission_data.hdf5"

src/sc_linac_physics/applications/field_emission/measurements.py:78

• find_dataframes() can crash when no cavities are selected (UnboundLocalError for dataset) or when a selected cavity/readout path is missing in the HDF5 file (KeyError). Returning empty results makes the GUI easier to handle and avoids hard crashes.

    cav_list = [i + 1 for i, c in enumerate(cav) if c]
    with h5py.File(h5_filename, "r") as h5f:
        dfs = {}
        for c in cav_list:
            filepath = f"CM{cm}/{stripped_date}/CAV{c}/{readout}"

src/sc_linac_physics/applications/field_emission/field_emission_gui.py:29

• These imports rely on the current working directory and will fail when the GUI is launched from elsewhere (e.g., via sc-linac). Use intra-package imports.

This issue also appears on line 30 of the same file.

from measurements import (
    match_measurement_dates,
    fetch_measurement_metadata,
    find_dataframes,
)

src/sc_linac_physics/applications/field_emission/field_emission_gui.py:39

• This GUI module is not referenced by the project's standard launcher mechanism (sc_linac_physics/cli/launchers.py has no launch_*field_emission*), so it cannot be discovered/started via sc-linac or a console script entrypoint.

class FieldEmission(Display):
    def __init__(self, parent=None):
        super().__init__(parent=parent)
        self.setWindowTitle("LCLS-II Field Emission")

src/sc_linac_physics/applications/field_emission/gui_sandbox.py:31

• These imports rely on the current working directory and will fail when the package is imported/installed. Use intra-package imports.

This issue also appears on line 32 of the same file.

from measurements import (
    match_measurement_dates,
    fetch_measurement_metadata,
    find_dataframes,
)

src/sc_linac_physics/applications/field_emission/gui_sandbox.py:276

• self._selected_rows[-1] is accessed before checking whether anything is selected, which can raise IndexError if the selection is empty (e.g., after clearing or if the user deselects).

        print(f"selected rows: {self._selected_rows}")
        # TODO - circular index??
        idx = self._selected_rows[-1]
        m = self._current_measurements[idx]

src/sc_linac_physics/applications/field_emission/amp_vs_time_from_csv.py:81

• This hard-coded absolute output path will fail for other users/environments and in CI. Consider making the output directory a CLI arg and/or using sc_linac_physics.utils.platform_paths so runs are portable.

This issue also appears on line 112 of the same file.

        fig.savefig(
            f"/Users/kvetta/sc-rad/cropped_comm/amp_plot_cm{cryomodule}_{timestamp}.png"
        )

src/sc_linac_physics/applications/field_emission/amp_vs_radiation_from_csv.py:58

• build_rad_readout_pvs() depends on a global rad_readout_type which is only defined inside the __main__ block. Calling this function from elsewhere (or refactoring) will raise NameError. Pass the readout type as an explicit parameter.

This issue also appears on line 125 of the same file.

def build_rad_readout_pvs(decarad, rad_channels):
    """choose radmon readout suffix depending on selection (instant vs average)"""
    rad_readout_pvs = []
    rad_prefix, _, _ = build_decarad_pvs(decarad)
    if rad_readout_type == "instant":

src/sc_linac_physics/applications/field_emission/amp_vs_time_from_csv.py:18

• This script references All Comm measurements by CM.csv, but that file does not exist in the repository. Running the script as-is will fail with FileNotFoundError unless the caller happens to have that file in their CWD.

input_csv = "All Comm measurements by CM.csv"

src/sc_linac_physics/applications/field_emission/amp_vs_radiation_from_csv.py:141

• This script writes output to a hard-coded absolute path, which is not portable across users/machines.

            aligned_time_data.to_csv(
                f"/Users/kvetta/sc-rad/radi/{rad_readout_type}/cm{cryomod}_"
                f"{stamp}_cavity{cav_num}_{rad_readout_type}.csv"
            )

src/sc_linac_physics/applications/field_emission/amp_vs_radiation_from_csv.py:122

• This script references All Comm measurements by CM.csv, but that file does not exist in the repository. Running the script as-is will fail with FileNotFoundError unless the caller has a local copy in their CWD.

    input_csv = "All Comm measurements by CM.csv"

src/sc_linac_physics/applications/field_emission/gui_sandbox.py:41

• gui_sandbox.py appears to be an experimental/alternate implementation alongside field_emission_gui.py, with substantial duplicated logic. Shipping both in the installed package increases maintenance surface and can confuse users about the supported entrypoint.

class FieldEmission(Display):
    def __init__(self, parent=None):
        super().__init__(parent=parent)
        self.setWindowTitle("LCLS-II Field Emission")

src/sc_linac_physics/applications/field_emission/field_emission_gui.py:379

• Leftover debug print() calls in GUI code can clutter stdout (especially when launched from sc-linac/PyDM). Consider removing or replacing with the project's structured logging utilities.

            print(f"CAVITY: {cav_num}")

[Copilot]

🟡 Changes recommended

Unresolved critical packaging, GUI, update-flow, and data-handling issues block approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (9)

src/sc_linac_physics/applications/field_emission/amp_vs_radiation.py:90

• fetch_pv_data() explicitly records handler.validities, but alignment ignores is_valid and uses every value. Invalid archiver samples consequently enter the persisted CSV/HDF5 data and the fit; filter each dataframe by its validity mask before constructing the series.

    for pv_name, df in dfs.items():
        s = pd.Series(df["values"].values, index=df["timestamps"])

src/sc_linac_physics/applications/field_emission/amp_vs_radiation.py:123

• This new end-to-end path builds the cavity/radiation PV lists, performs the archiver queries, aligns timestamps, and writes all CSVs, but the PR has no tests for it. Add mocked-archiver coverage for PV naming, both readout suffixes, empty/invalid data, and output filenames so this data pipeline cannot silently produce unusable HDF5 input.

def generate_amp_vs_rad_csvs(cm, start, end, decarad):
    """combine methods for portable amplitude and radiation generation"""
    print(f"Processing CM{cm} {start} -> {end}")
    csv_date = start.strftime(CSV_DATE_FORMAT)
    amp_pvs = build_amplitude_pvs(cm)

src/sc_linac_physics/applications/field_emission/field_emission_gui.py:65

• Validation exceptions from validate_emission_data() escape the Qt clicked slot here. A typo in a CM, date, Decarad, eLog, or filter field therefore produces no in-dialog feedback and can leave the update dialog/application in an error state; catch ValueError and show the message while keeping the input dialog available for correction.

        if dialog.exec():
            single_update(dialog.get_inputs())

src/sc_linac_physics/applications/field_emission/field_emission_gui.py:480

• After a successful update, this method returns without refreshing _current_measurements or the list widget. Because the cryomodule selection has not changed, the newly written measurement remains invisible until the user switches away and back. Have the update dialog report success and reload the current cryomodule's measurements.

    def open_update_dialogue(self):
        dialog = UpdateButtons()
        dialog.exec()

src/sc_linac_physics/applications/field_emission/gui_updater.py:140

• After the start date is parsed, an invalid end date is parsed outside the try block, so one malformed row aborts the entire multi-CM update with ValueError instead of being skipped like malformed start rows. Handle both end-date branches in the same validation path.

            if row[3] == "":
                end_date = datetime.strptime(
                    f"{row[1]} {row[4]}", STANDARD_DATE_FORMAT
                )
            else:
                end_date = datetime.strptime(
                    f"{row[3]} {row[4]}", STANDARD_DATE_FORMAT

src/sc_linac_physics/applications/field_emission/gui_updater.py:22

• The validation helpers intentionally raise ValueError for malformed form input, but this Qt slot calls single_update() without catching those errors or showing them to the user. An empty or mistyped field therefore escapes the event handler instead of leaving the dialog available for correction. Catch validation failures and present an error without discarding the input.

def single_update(input_row):
    valid = validate_emission_data(input_row)

src/sc_linac_physics/applications/field_emission/plot_me.py:47

• This fit relationship determines the C1/C2 values shown to operators, but the docstring gives no source or provenance for the 2.5 exponent and exponential form. Add the reference used for this field-emission model before treating these fitted values as analysis results.

def fit_equation(amp, c1, c2):
    """fit line equation currently y = C1(E0)^(2.5) * exp(-C2/E0)"""
    return c1 * (amp**2.5) * np.exp((-c2) / amp)

src/sc_linac_physics/applications/field_emission/plot_me.py:66

• When fitting, ax.legend(handles=fit_patches, ...) replaces the scatter legend, while each fit patch label contains only C1/C2 and not the channel label passed into add_poly_fit. The fit legend therefore cannot identify which coefficients belong to Ch 1, Ch 2, etc.; include label in the patch label or retain channel handles.

    patch = mpatch.Patch(
        color=color, label=f"C1: {param[0]:.1g}    C2: {param[1]:.1f}"

src/sc_linac_physics/applications/field_emission/update_h5py.py:145

• require_dataset() only reuses an existing dataset when its shape and dtype match; it does not replace or resize it. Re-running an update for the same CM/date/readout with a different number of archive samples therefore raises before writing, so the HDF5 record cannot be refreshed. Delete/recreate or resize the existing dataset deliberately.

    dset = group.require_dataset(
        f"{readout}",
        shape=values.shape,
        dtype=values.dtype,
        compression="gzip",
    )
    dset[...] = values

• Files reviewed: 12/15 changed files
• Comments generated: 11
• Review effort level: Lite