Add pulse timings dataclass - #4497
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4497 +/- ##
==========================================
- Coverage 49.30% 49.21% -0.09%
==========================================
Files 151 152 +1
Lines 29611 29836 +225
==========================================
+ Hits 14599 14684 +85
- Misses 15012 15152 +140 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…operties for PF coil active phases
…roved timing management
…d timing management
…mproved pulse timing management
…nd integrate point abbreviations for improved clarity in timing management
…ings dataclass for improved clarity in timing representation
| return len(self.total_pulse_cumulative) | ||
|
|
||
| @property | ||
| def n_pulse_points_intervals_total(self) -> int: |
There was a problem hiding this comment.
This doesn't seem to be getting used anywhere ?
| point_abbreviations: ClassVar[tuple[str, ...]] = ( | ||
| "BOP", | ||
| "EOR", | ||
| "BOF", | ||
| "EOF", | ||
| "EOP", | ||
| "Dwell", | ||
| ) | ||
|
|
||
| point_labels: ClassVar[tuple[str, ...]] = ( | ||
| "Coil precharge", | ||
| "$I_{\\text{p}}$ Ramp-Up", | ||
| "Fusion ramp", | ||
| "Burn", | ||
| "$I_{\\text{p}}$ ramp-down", | ||
| "Dwell", | ||
| "Restart pulse", | ||
| ) |
There was a problem hiding this comment.
Since these are constants, can they be moved to the top of this file instead please?
There was a problem hiding this comment.
I believe it is best to keep them as class variables contained withing the dataclass. Meaning everything is kept in one place. I have added a __post_init__ that checks that the number of labels always matches the number of points in the dataclass. This also means everything is in one place and checked if we ever want to add or change the time points
There was a problem hiding this comment.
This makes sense to keep them here. Could the variable names be capitalised please? Since they are still behaving as constants, so would make it clearer to a user
…naming and enhance validation in PulseTimings dataclass
Co-authored-by: clmould <86794332+clmould@users.noreply.github.com>
clmould
left a comment
clmould
left a comment
There was a problem hiding this comment.
Just 1 small change
Also could you fix the pre-commit failure too please?
| point_abbreviations: ClassVar[tuple[str, ...]] = ( | ||
| "BOP", | ||
| "EOR", | ||
| "BOF", | ||
| "EOF", | ||
| "EOP", | ||
| "Dwell", | ||
| ) | ||
|
|
||
| point_labels: ClassVar[tuple[str, ...]] = ( | ||
| "Coil precharge", | ||
| "$I_{\\text{p}}$ Ramp-Up", | ||
| "Fusion ramp", | ||
| "Burn", | ||
| "$I_{\\text{p}}$ ramp-down", | ||
| "Dwell", | ||
| "Restart pulse", | ||
| ) |
There was a problem hiding this comment.
This makes sense to keep them here. Could the variable names be capitalised please? Since they are still behaving as constants, so would make it clearer to a user
3 participants
This pull request refactors how pulse timing and cumulative time points are handled throughout the codebase by introducing and consistently applying the
PulseTimingsclass. This change improves code clarity, reduces duplication, and centralizes pulse timing logic, making it easier to maintain and extend. The update affects plotting routines, model calculations, and output formatting for pulse-related data.Pulse timing refactor and centralization:
Replaced manual cumulative time calculations in plotting functions (e.g.,
plot_current_profiles_over_time,plot_system_power_profiles_over_time,plot_cs_stress_time_profile) with usage of the newPulseTimingsclass, ensuring consistent time axis generation and annotation across all plots.Updated model logic in
physics.pyandpfcoil.pyto usePulseTimingsfor all derived pulse timing values, such as total pulse time, plasma-present time, and no-burn time, removing duplicated cumulative sum logic.Modified power system models (
power.py) to passPulseTimingsas an argument to the PF coil power calculation routines (pfpwr), ensuring all power calculations use unified pulse timing data.Output and labeling improvements:
Standardized the use of pulse time labels and cumulative time points in output routines and plot annotations, using properties from
PulseTimingsfor axis tick labels and time column headers.Cleanup and removal of legacy code:
times_variables.pyand related model files, reducing code redundancy.Checklist
I confirm that I have completed the following checks: