Skip to content

Conversation

@proy30
Copy link
Member

@proy30 proy30 commented Jun 10, 2025

This PR provides the user guidance on which inputs are invalid. Additionally, the PR refactors the current logic to track such errors.

Changes:

  • A new dynamic notification banner in the Inputs toolbar which will display the count of input errors the user has. Hovering over the banner will tell the user exactly which inputs are incorrect along with the error message.
  • A new class, SimulationValidation in validation.py, is created to encapsulate all logic for determining whether the simulation can run based on current input errors.
  • The primary method in the class, update, takes the input section header as an argument.
    • This allows the method to check only the relevant section for errors.
  • Add a JS function to suppress resizeWarning error.
    • Does not affect the dashboard functionality.
  • Remove unhelpful (and unneeded) comments in generalFunctions.py
  • Fix variable naming in shared.py
    • A variable referring to csr inputs was incorrectly labeled under space charge

Demo:

chrome_u8cxvRN2ps.mp4

Merge after #981

@proy30 proy30 requested a review from Copilot June 10, 2025 01:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds dynamic input error tracking to the dashboard, introduces a centralized SimulationValidation class to manage error state per section, and suppresses unnecessary ResizeObserver console errors.

  • Add an error banner in the Inputs toolbar showing total and detailed input errors.
  • Introduce SimulationValidation to replace piecemeal validation updates with a section-based update API.
  • Suppress redundant ResizeObserver loop errors in custom.js.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/python/impactx/dashboard/Toolbar/sim_history/custom.js Suppress ResizeObserver errors by overriding console.error.
src/python/impactx/dashboard/Toolbar/controls.py Add error_notification banner and include it in the toolbar.
src/python/impactx/dashboard/Input/validation.py Create SimulationValidation to track and cache per-section errors.
src/python/impactx/dashboard/Input/space_charge_configuration/spaceChargeMain.py Swap DashboardValidation.update_* calls for SimulationValidation.update.
src/python/impactx/dashboard/Input/shared.py Update imports and use SimulationValidation.update in shared utilities.
src/python/impactx/dashboard/Input/latticeConfiguration/variable_handler.py Replace DashboardValidation calls; incorrectly calls update_simulation_validation_status.
src/python/impactx/dashboard/Input/latticeConfiguration/latticeMain.py Replace validation triggers with SimulationValidation.update.
src/python/impactx/dashboard/Input/inputParameters/inputMain.py Remove old validation hook but omit new SimulationValidation.update call.
src/python/impactx/dashboard/Input/generalFunctions.py Remove unneeded comments and extend get_default with robust fallback.
src/python/impactx/dashboard/Input/distributionParameters/distributionMain.py Hook distribution parameter changes into SimulationValidation.update.
src/python/impactx/dashboard/Input/init.py Export SimulationValidation alongside DashboardValidation.
Comments suppressed due to low confidence (3)

src/python/impactx/dashboard/Input/validation.py:173

  • The if n_cell_error block is mis-indented outside the loop, so only the last direction is ever checked. Move the if and its append logic inside the for block.
        for direction in ["x", "y", "z"]:

src/python/impactx/dashboard/Input/inputParameters/inputMain.py:44

  • You removed the validation hook but did not add a SimulationValidation.update("Input Parameters") call. Without it, changes here won’t update the input-error state.
        if state.space_charge not in current_sc_list:

src/python/impactx/dashboard/Input/validation.py:158

  • [nitpick] New validation logic in SimulationValidation and the toolbar banner lack unit tests. Consider adding tests for update behavior and InputToolbar.error_notification rendering.
class SimulationValidation:

current_sc_list = ui_props.get("space_charge_list", [])
if state.space_charge not in current_sc_list:
state.space_charge = current_sc_list[0]
DashboardValidation.update_simulation_validation_status()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine to not have to update the simulation with tracking_mode because tracking_mode is a dropdown and will always have a valid input.

@ax3l ax3l added the component: dashboard our browser based trame dashboard label Jun 23, 2025
@proy30 proy30 force-pushed the dashboard/add_input_errors_tracker branch 2 times, most recently from 68527e7 to a270010 Compare June 26, 2025 17:41
@proy30 proy30 force-pushed the dashboard/add_input_errors_tracker branch from d03f1b5 to caa96bf Compare July 17, 2025 02:26
@proy30 proy30 force-pushed the dashboard/add_input_errors_tracker branch from e3e1f41 to f269f1a Compare July 18, 2025 00:16
@proy30 proy30 force-pushed the dashboard/add_input_errors_tracker branch from f269f1a to a5db113 Compare July 18, 2025 00:23
proy30 added 19 commits August 7, 2025 17:37
break into multiple smaller functions
docstring, type hint, error messages
docstring/simplify
because the input of state.kin_energy_unit is coming from a dropdown, its input is a string. if it goes through the shared.py state change '@state.change(*INPUT_DEFAULTS)', then it will be validated as a string. Because it has no error messages (technically because the state.kin_energy_unit default is 'MeV' and would be a valid python identifier), it will be converted to a numeric value. however the 'convert_to_numeric' method will set the value to 'None' if it can't be turned into a default type of int or float. this will set the state.kin_energy_unit to 'None' and the dashboard will not launch. The solution to this is not have any dropdown inputs go through @state.change(*INPUT_DEFAULTS) because every dropdown will have a valid set of inputs by default.
which also fixes distribution validaiton
proy30 and others added 23 commits August 7, 2025 17:38
The current default for lattice name is 'None', a string. We change that to a python literal None now. But because that is the case the name parameter for the lattice element will be empty. So we need to make sure in the dashboard test we provide a name for the element so that there are no inputs errors before running a simulation
doesnt affect dashbaord visually or functionally

this resizeObserver error message appears whenever hovering over a tooltip that takes the entire width of the page, or if we hover over the tooltip showing input errors. not exactly sure why and this is just a suppression 'bandaid'
…g errors

correctly track n_cell and blocking factor

completely refactor

refactor fixup

create SimulationValidation class

this is where the logic of checking for input errors is now stored

update main function name

shorten because the class name helps with identifying

call correct function for variables

add type hints

simplify

update 'get_default'

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

update var name for disabling sim
place in a new folder 'Input/validation'
the set_js_input is going to reach the 'exception' case if I place an invalid error. If I am intentionally trying to set an error, such as if I am testing the dashboard validation with an error input, I need it to fail on the dashboard end only.
sets all inputs to an error state, checks to make sure that the # of input errors is accurate, then reset different parts of the dashboard and continuously check the number of errors.
it always returned true
create '_apply_state_updates'
@proy30 proy30 force-pushed the dashboard/add_input_errors_tracker branch from a5db113 to 4ca3e8c Compare September 19, 2025 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: dashboard our browser based trame dashboard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants