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

Refactor Unit Handling for kin_energy #806

Conversation

proy30
Copy link
Contributor

@proy30 proy30 commented Jan 18, 2025

This PR refactors the implementation for handling kinetic energy unit changes. While initially intended to address broken functionality introduced in #780, the scope has evolved to implement a more intuitive approach for unit handling that no longer depends on the previous logic.

Previously, changing units would update both the UI display and backend values to represent the same physical quantity (e.g., 5 MeV changed to GeV would result in 0.005 on both the UI and backend). This behavior has now been redesigned to better align with user expectations. With this update, the backend value (in MeV) is updated to reflect the new unit, while the UI number remains constant (e.g., 5 MeV → 5 GeV).

@proy30 proy30 added the component: dashboard our browser based trame dashboard label Jan 18, 2025
@proy30
Copy link
Contributor Author

proy30 commented Jan 18, 2025

before:

chrome_mGqIWXmYW2.mp4

after (print statement only used in testing):

Code_ucXHq6FUm5.mp4

@proy30 proy30 requested a review from ax3l January 21, 2025 20:57
@ax3l
Copy link
Member

ax3l commented Jan 22, 2025

@cemitch99 Oh, did we want to change this? I liked that it flipped so nicely :)

@ax3l ax3l requested a review from cemitch99 January 22, 2025 18:45
@ax3l
Copy link
Member

ax3l commented Jan 22, 2025

Ok. The rationale is to keep it simple and consistent, that is ok :)

@ax3l ax3l merged commit dd79d2f into ECP-WarpX:development Jan 22, 2025
15 of 16 checks passed
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