Repository navigation
Conversation
| return theta, phi, r | ||
|
|
||
| def __str__(self) -> str: | ||
| def __str__(self, human_readable: bool = False) -> str: |
There was a problem hiding this comment.
I would simply replace current template with human readable, instead of adding a flag to control this behaviour
There was a problem hiding this comment.
Pull request overview
Adds human-readable annotations to some generated simulator input files and updates test fixtures/expected outputs accordingly, improving the readability of generated configs across converters.
Changes:
- Add inline/unit comments to generated TOPAS config template and update corresponding TOPAS test expectation.
- Add header/comment lines to FLUKA beam/beampos output and update expected FLUKA output fixture.
- Introduce a
human_readablemode for SHIELD-HIT12Abeam.datgeneration and adjust generation call site; minor formatting tweak in Geant4 macro output.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/topas/test_topas_converter.py | Updates expected TOPAS config string to include new inline comments. |
| tests/fluka/expected_fluka_output/fl_sim.py | Updates expected FLUKA output to include newly added comment headers. |
| converter/topas/config.py | Adds inline human-readable comments to TOPAS config template lines. |
| converter/shieldhit/parser.py | Switches beam.dat generation to a human-readable-aware string rendering call. |
| converter/shieldhit/beam.py | Adds a second (human-readable) beam.dat template and a flag-controlled rendering path. |
| converter/geant4/Geant4MacroGenerator.py | Splits combined max/min GPS energy commands into separate macro lines. |
| converter/fluka/cards/beam_card.py | Adds comment headers describing BEAM/BEAMPOS columns in generated FLUKA input. |
| converter/common.py | Introduces a human_readable attribute on the base Parser. |
Comments suppressed due to low confidence (1)
converter/shieldhit/beam.py:135
- There are now two large templates (
beam_dat_template_rawandbeam_dat_template) that are mostly duplicated. This increases the risk of future drift when editing one but not the other. Consider generating the human-readable comments conditionally from a single canonical template (or annotating lines in a post-processing step).
beam_dat_template_raw: str = """
RNDSEED 89736501 ! Random seed
JPART0 {particle} ! Incident particle type{particle_optional_comment}
{optional_heavy_ion_line}
TMAX0 {energy} {energy_spread} ! Mean and spread (1 sigma) of primary particle kinetic energy [{energy_unit}]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def __str__(self, human_readable: bool = False) -> str: | ||
| """Return the beam.dat config file as a string.""" | ||
| theta, phi, _ = BeamConfig.cartesian2spherical(self.beam_dir) |
There was a problem hiding this comment.
BeamConfig.__str__ now takes an extra human_readable argument. Python’s __str__ special method is expected to be parameterless (__str__(self)), and adding parameters encourages unconventional direct calls to .__str__(...) and can confuse tooling/type checkers. Consider keeping __str__ parameterless and adding a dedicated method (e.g. to_string(human_readable: bool = False)) for the alternate formatting.
| configs_json = super().get_configs_json() | ||
| configs_json.update( | ||
| { | ||
| "beam.dat": str(self.beam_config), | ||
| "beam.dat": self.beam_config.__str__(human_readable=self.human_readable), | ||
| "mat.dat": self.geo_mat_config.get_mat_string(), |
There was a problem hiding this comment.
This directly calls BeamConfig.__str__(human_readable=...) to control formatting. That works, but it depends on a nonstandard __str__ signature and bypasses the normal str(obj) convention. Prefer an explicit formatter method on BeamConfig (and keep __str__ parameterless) so call sites are clearer and less surprising.
| "label": "", | ||
| "simulator": "", | ||
| } | ||
| self.human_readable = False |
There was a problem hiding this comment.
A human_readable flag is introduced on the base Parser, but there’s no code in the repo that sets it (no CLI option, no parsing from input JSON, and no use in run_parser). As a result, the human-readable mode can’t be enabled through the current public entrypoints. Consider wiring this through the CLI/API or removing it until it’s supported end-to-end.
| self.human_readable = False |
| NUCRE {nuclear_reactions} ! Nuclear Reactions: 1-ON, 0-OFF | ||
| {optional_beam_modulator_lines} | ||
| BEAMPOS {pos_x} {pos_y} {pos_z} ! Beam Source Position (X, Y, Z) [cm] | ||
| BEAMDIR {theta} {phi} ! Beam Direction (Theta, Phi) [deg] |
There was a problem hiding this comment.
you add redundant, but important for humans reader information, namely which is the directional vector of the beam ?
humans may ponder whether beam goes along Z axis or not ?
this is part of SHIELD-HIT12A manual:
this is part of user documentation:
please note that in the UI user defines beam direction as a directional vector
this information is transformed into theta-phi angles
theta-phi angles are saved into this beam.dat file - that is fine
directional vector (typed in UI by user) is not, but it could be added as a comment
such redundancy increases a lot readability
| d:So/Demo/BeamAngularSpreadX = 0.0032 rad | ||
| d:So/Demo/BeamAngularSpreadY = 0.0032 rad | ||
| i:So/Demo/NumberOfHistoriesInRun = 10000 | ||
| i:So/Demo/NumberOfHistoriesInRun = 10000 # Number of Particles |
There was a problem hiding this comment.
| i:So/Demo/NumberOfHistoriesInRun = 10000 # Number of Particles | |
| i:So/Demo/NumberOfHistoriesInRun = 10000 # Number of primary particles |
| DEFAULTS HADROTHE | ||
| * PROTON beam of energy 0.07 GeV | ||
| * flat circular shape with max radius=3.0 cm, min radius=0.0 cm | ||
| * Energy Spread Rmax Rmin Div Particle |
There was a problem hiding this comment.
this file has so called fixed format, inherited from Fortran punched cards.
if you document each of the numbers in the format, please follow the format and keep proper spacing
No description provided.