Repository navigation
Parse multiply shortcuts that look like multigroup ZAIDs - #1046
SpaceOceanCorp wants to merge 1 commit into
Conversation
A multiply such as 1000.00m has the shape of a multigroup ZAID, so the lexer made it a ZAID and the parser rejected it. The lexer can't tell which one it is: 1001.50m is a valid ZAID on a material. Lex these as NUM_MULTIPLY, and let zaid_phrase accept a NUM_MULTIPLY that has the shape of a multigroup ZAID. File names accept NUM_MULTIPLY too, since they accepted ZAID.
MicahGale
added
bugs
MicahGale
left a comment
MicahGale
left a comment
There was a problem hiding this comment.
This looks pretty good. I just have one suggestion to make the test a little cleaner.
| if hasattr(p, "NUM_MULTIPLY") and not self._MULTIGROUP_ZAID.fullmatch(p[0]): | ||
| self.error(p._slice[0]) |
There was a problem hiding this comment.
While it is theoretically possible to use a multiply shortcut inside of a material definition, it is a very contrived scenario that requires no use of libraries in the ZAIDs, and requires multiplications to go from ZAID to fraction and back repeatedly. I think only I would come up with something so contrived, so we can exclude this edge case.
| def test_multigroup_zaid_in_material(text, libraries): | ||
| material = montepy.Material(Input([text], BlockType.DATA)) | ||
| assert [str(nuclide.library) for nuclide, _ in material] == libraries | ||
| assert material.format_for_mcnp_input((6, 3, 0)) == [text] |
There was a problem hiding this comment.
use mcnp_str to test here as it's cleaner.
MicahGale
left a comment
MicahGale
left a comment
There was a problem hiding this comment.
A little more coverage is needed.
| @_("ZAID", "ZAID padding") | ||
| @_("ZAID", "ZAID padding", "NUM_MULTIPLY", "NUM_MULTIPLY padding") | ||
| def zaid_phrase(self, p): | ||
| if hasattr(p, "NUM_MULTIPLY") and not self._MULTIGROUP_ZAID.fullmatch(p[0]): |
There was a problem hiding this comment.
There are two lines that were added here which were not covered: https://coveralls.io/jobs/190904452. Please investigate and see if it feasible to get 100% patch coverage.
Pull Request Checklist for MontePy
Description
vol 2 1000.00mraised aParsingError, because1000.00mhas the shape of a ZAID with a multigroup library and the lexer returnedZAID. The lexer can't decide this alone, since1001.50mis a valid ZAID on a material, so the decision moves to the parser:ZAIDtoken no longer matches a two digit library with themsuffix. These now lex asNUM_MULTIPLY, so all multiply shortcuts lex the same way.zaid_phrasealso acceptsNUM_MULTIPLY, and reports a syntax error unless it has the shape of a multigroup ZAID. Som1 1001.50m 1.0still works, andm1 1.5m 1.0andvol 1.5m 2are still errors.file_atomacceptsNUM_MULTIPLY, because it acceptedZAID. Without thisread file=1001.50mwould stop working. As a side effectread file=1.5mnow parses too.This also fixes the intermittent failure of
test_multiply_fortran_floatson the example0000.00m. That test is unchanged.One existing behavior I left alone:
vol 1000.00m 2parses as an isotope fraction, as it did before, becauseDataParserdoesn't check which card a ZAID is on.The new tests cover multiplies shaped like ZAIDs, multigroup ZAIDs on materials including mixed libraries, the two error cases, and file names. The multiply tests and
read file=1.5mfail ondevelop; the others pin behavior that has to stay the same.Fixes #1045
General Checklist
blackversion 25 or 26.LLM Disclosure
Are you?
Were any large language models (LLM or "AI") used in to generate any of this code?
Documentation Checklist
.. versionchanged::or.. versionadded::directives.N/A, no public API changes.
Infrastructure Changes
N/A, no infrastructure changes.
Significant features or Behavior changes
N/A, bug fix.
First-Time Contributor Checklist
pyproject.tomlif you wish to do so.Additional Notes for Reviewers
Ensure that:
📚 Documentation preview 📚: https://montepy--1046.org.readthedocs.build/en/1046/