50.1 parameter workflow #133
Conversation
…ing with extract_resonance_entrries()
… treatment and spin group numbers
…tFile, and SammyParameterFile
…to sammy/io/card_formats/par10_isotopes.py
…sentation. becuase it was annoying!
There was a problem hiding this comment.
Pull Request Overview
Adds support for reading and writing SAMMY parameter files via a new ParManager and migrates isotope parsing to a dedicated Card10 class.
- Replaces legacy
IsotopeCardwithCard10in tests and I/O code - Introduces
ParManagerfor high-level parameter file operations - Updates models for optional isotope information and improves logging
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/pleiades/sammy/parameters/test_isotope.py | Updated imports/tests to use Card10 instead of IsotopeCard |
| tests/unit/pleiades/sammy/io/test_par_manager.py | Added placeholder file for ParManager unit tests |
| tests/unit/pleiades/sammy/io/card_formats/test_par10_isotopes.py | New unit tests for Card10 parsing of isotope card |
| src/pleiades/sammy/parameters/isotope.py | Removed deprecated IsotopeCard implementation |
| src/pleiades/sammy/parameters/init.py | Dropped import of the removed isotope module |
| src/pleiades/sammy/io/par_manager.py | Added ParManager class for managing SAMMY param file I/O |
| src/pleiades/sammy/io/card_formats/par10_isotopes.py | Introduced Card10 for parsing/writing Card 10 entries |
| src/pleiades/nuclear/models.py | Made isotope_information field optional in IsotopeParameters |
| src/pleiades/nuclear/isotopes/models.py | Removed unused __str__ override |
| src/pleiades/nuclear/isotopes/manager.py | Enhanced logging, fixed prints, and added isotope lookup |
| examples/samexm/ex012/ex012a.inp | Added directive for quantum numbers in example input file |
Comments suppressed due to low confidence (2)
src/pleiades/nuclear/isotopes/manager.py:123
- Method name typo:
get_istotpe_info_from_massshould beget_isotope_info_from_mass.
def get_istotpe_info_from_mass(self, mass: float) -> Optional[IsotopeInfo]:
tests/unit/pleiades/sammy/io/test_par_manager.py:1
- Test file contains no actual test functions—add assertions for
ParManagermethods likeextract_isotopes_and_abundances.
"""Unit tests for SAMMY parameter file classes."""
…ke the pre-commit.ci overlords happy.
…d for pleiades examples.
for more information, see https://pre-commit.ci
…rs and validation
…d extended formats
…tation for spin groups and their channels
…f spin groups and channels
…dation of spin groups and channels
…ding, extraction methods, and data consistency
…s parameter spin groups by extracting spin group numbers
…inGroupChannels and validate spin group initialization
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Hey @KedoKudo, I was able to wrap up all the needed pytests for the parameter file io modules. Just submitted it for review. |
Thanks, I will start the review process |
| logger.error(message) | ||
| raise ValueError(message) | ||
|
|
||
| # if fit_config is not an instance of FitConfig, raise an error |
There was a problem hiding this comment.
I think this is a good advice, we only need to check fit_config once.
| for line in content_lines: | ||
| # Check if the line has a continuation marker ("-1") at the end | ||
| # If it does, append the line to the current isotope group | ||
| if line.endswith("-1"): |
There was a problem hiding this comment.
A correctly formatted parameter files should not have white space after -1, but I think maybe adding rstrip() will make it more robust in case an extra space is added accidentally
| if fit_config.nuclear_params.isotopes is None: | ||
| logger.info(f"Isotpe list is empty, creating new isotope with mass {mass}") |
There was a problem hiding this comment.
I recommend accepting this suggestion.
| line_key = line.strip()[:5].upper() | ||
| for header, card_enum in PAR_HEADER_MAP.items(): | ||
| header_key = header.strip()[:5].upper() | ||
| if line_key == header_key: | ||
| found_cards.add(card_enum) |
There was a problem hiding this comment.
Feel free to ignore this one, we already know that SAMMY only check the first 5 char to identify card
…rd formats. Moved all imports to top of file
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
for more information, see https://pre-commit.ci
…eyword_pairs_to_dict function
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive parameter file manager for SAMMY with complete read/write functionality for handling SAMMY parameter files based on FitConfig objects. The implementation includes reading and writing capabilities for various parameter cards including resonance entries, isotope parameters, radius data, normalization data, background data, particle pair definitions, and spin group definitions.
Key changes include:
- Implementing ParManager class for handling SAMMY parameter file I/O operations
- Creating modular card format parsers for different parameter types (par01-par10, inp04, inp10)
- Replacing the legacy isotope parameter handling with new card-based approach
- Adding comprehensive unit tests for all card formats and the parameter manager
Reviewed Changes
Copilot reviewed 36 out of 38 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/pleiades/sammy/test_parfile.py | Removed legacy parameter file test module |
| tests/unit/pleiades/sammy/parameters/test_isotope.py | Updated imports to use new card format classes |
| tests/unit/pleiades/sammy/io/test_par_manager.py | Added comprehensive tests for ParManager functionality |
| tests/unit/pleiades/sammy/io/card_formats/ | Added unit tests for all card format parsers |
| tests/unit/pleiades/nuclear/test_nuclear_models.py | Updated to use new SpinGroups and SpinGroupChannels models |
| src/pleiades/utils/helper.py | Added pseudo-scientific notation parser for SAMMY files |
| src/pleiades/sammy/parameters/ | Removed legacy isotope parameter handling |
| src/pleiades/sammy/io/par_manager.py | Added main ParManager class for parameter file operations |
| src/pleiades/sammy/io/card_formats/ | Added modular card format parsers for different parameter types |
| # self.assertEqual(params.isotopes[0].resonances, [resonance_entry]) | ||
| # self.assertEqual(params.isotopes[0].radius_parameters, [radius_params]) |
There was a problem hiding this comment.
Commented-out assertions should be removed or uncommented with proper implementation. These dead code comments reduce code clarity and should be cleaned up.
| # self.assertEqual(params.isotopes[0].resonances, [resonance_entry]) | |
| # self.assertEqual(params.isotopes[0].radius_parameters, [radius_params]) | |
| self.assertEqual(params.isotopes[0].resonances, [resonance_entry]) | |
| self.assertEqual(params.isotopes[0].radius_parameters, [radius_params]) |
Copilot uses AI. Check for mistakes.
| for channel in channels: | ||
| lines.append(f" Chan= {channel:2d},") | ||
|
|
||
| # Remove trailing blank lines (but keep one at end) |
There was a problem hiding this comment.
[nitpick] The comment mentions removing trailing blank lines but keeping one at the end, however the implementation removes all trailing blank lines and then adds one. Consider clarifying the comment to match the implementation.
| # Remove trailing blank lines (but keep one at end) | |
| # Remove all trailing blank lines and then add one to ensure exactly one blank line at the end |
Copilot uses AI. Check for mistakes.
Working on parameter file manager to read in or write SAMMY parameter files based FitConfig objects.
This functionality with be based on creating a ParManager that will be passed a parameter file path and a FitConfig object.
Read functionality - Taking a parameter file and populating a given FitConfig object.
Write functionality - Taking a given FitConfig object and writing a parameter file.
Write unit tests for par_manager and card_formats.