Repository navigation
Fix/mec state names - #30
Merged
Merged
Conversation
The 10-byte state-name field is padded, but not always with the same byte:
some writers pad with spaces and others with NULs, and older files leave it
blank altogether, having never stored the names. mec_load did
statename.split()[0], which assumed space padding and at least one token, so
a blank field failed in one of two ways depending on how it was padded:
space-padded split() returns [], so [0] raised IndexError and the file
could not be loaded at all
NUL-padded split() does not treat NUL as whitespace, so the name came
back as ten NUL bytes - the file loaded, but every state was
named out of unprintable characters, which is worse than
failing because it is silent
Both now resolve: NUL is treated as padding, and a field that is blank after
that falls back to a positional name, S1..Skstat, so the states stay
distinguishable. Files that do store their names are unaffected.
Found on two 2005 AChR mechanism files, one padded each way, where it also
left the mechanism's cycle unusable: Cycle holds state names, so nine states
named alike cannot define one. With positional names the cycle resolves and
checks out at exactly 1.000000 in every record of both files.
Regression: 12 .mec files, 351 records, 349 load. The two that do not are
records 3 and 22 of QmechRL.mec, which fail differently and already did -
their rates refer to state indices beyond the Q matrix allocated from kstat.
That is a separate problem and is untouched here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit fixed blank and NUL-padded state-name fields in mec_load
but had no tests. Its parsing is moved, unchanged, into _state_name(field,
number) so that the rule can be tested without a .mec file:
- padding is removed whether it is spaces, NULs or both;
- no name ever contains a NUL;
- a blank field gives S<number>, the state's number in the file - which is
also how the drawing stored in the file numbers it, so S4 is the state
drawn as 4.
Four more tests load real files when SCALCS_MEC_DIR points at a folder of
them, and skip otherwise: DREMIS01.MEC, which stores no names; qmechs.mec,
whose names are NUL-padded; and qmechsDC1.mec, which names its open states
and leaves the shut ones blank. On master, before the fix, all four failed:
DREMIS01.MEC loaded with every state named with ten NUL bytes, and so with
no layout, since positions keyed by name could not tell the states apart.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.