Skip to content

Fix .inp unit substring checks overwriting kcal, m**3/kg and kg/m**3 - #169

Open
djkees wants to merge 1 commit into
nasa:mainfrom
djkees:up/inp-unit-substring-overwrite
Open

djkees wants to merge 1 commit into
nasa:mainfrom
djkees:up/inp-unit-substring-overwrite

Conversation

@djkees

@djkees djkees commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Summary

parse_embedded in source/input.f90 picks units by checking substrings in sequence, with each later match overwriting the earlier one. In three places a shorter substring was checked after a longer one that contains it, so correctly written units were overwritten by the wrong unit (1000x errors, no warning, exit code 0). This change checks the shorter substring first so the longer unit wins.

Issue: djkees#306

Originally fixed and merged in the djkees/cea fork as djkees#307; this is the same commit cherry-picked onto nasa/cea main.

Changes

  • h and u: check c (cal/mole) before kc (kcal/mole), so kcal is no longer read as cal.
  • v: check g (cm3/g) before kg (m3/kg).
  • rho: check g (g/cm3) before kg (kg/m3).
  • Added regression cases to test_embedded in source/input_test.pf: h(kcal), h,kcal, u(kcal), v(m**3/kg), rho(kg/m**3), rho,kg/m**3, plus the forms that already worked (h,cal, h,kj, v(cm**3/g), rho(g/cc)).

Why some syntaxes were affected and others not: the problem-section scanner replaces commas with spaces, so v,m**3/kg=1 passes m**3/kg straight to convert_units_to_si (exact match) and never reaches parse_embedded; only the parenthesis form v(m**3/kg) / rho(kg/m**3) did. The reactant-section scanner keeps commas, so h,kcal and h(kcal) both go through parse_embedded. The same applies to reactant rho,kg/m**3, which was also misread, although reactant density is currently parsed but not used in any calculation.

t, p and tces were checked for the same pattern and are not affected: none of their accepted unit names (k/r/c/f; bar/atm/psi/psia/mmhg) contains another's match string.

This is a minimal interim fix. The planned units rework (exact name matching against one reference table, see #121 and #124) would remove this class of bug entirely; parse_embedded and convert_units_to_si are not restructured here.

Testing

  • CTest (pFUnit, Release, gfortran, C bindings on), on this branch: 15/15 passed; pFUnit reports OK (122 tests), including test_embedded.
  • The new test_embedded cases fail against the unfixed code (expected: <"kcal/mole">) and pass with the fix.
  • Legacy CLI tests (test/main_interface/test_main.py): 14/14 passed.
  • End-to-end with the built cea.exe, before -> after:
Input Before After
h,kcal=-17.9 (HP, CH4/O2) T = 3441.48 K T = 3350.56 K
h(kcal)=-17.9 3441.48 K 3350.56 K
h,cal=-17900 3350.56 K 3350.56 K
h,kj=-74.8936 3350.56 K 3350.56 K
v(m**3/kg)=1 (TV, H2/Air, 3000 K) Density 0.1000E+4 kg/m^3 0.1000E+1
rho(kg/m**3)=1 0.1000E+4 0.1000E+1
v,m**3/kg=1 0.1000E+1 0.1000E+1
rho,kg/m**3=1 0.1000E+1 0.1000E+1

Compatibility / Numerical behavior

  • No expected changes to numerical results
  • Expected changes (explain and provide validation)

Results change only for inputs that were misread before: h/u given in kcal (either syntax), and v(m**3/kg) / rho(kg/m**3) in the parenthesis syntax (plus reactant rho in kg/m**3, which is parsed but unused). Inputs that were already read correctly are unaffected: .out files for all 14 legacy examples and samples/rp1311_examples.inp are byte-identical between unmodified nasa/cea main and this branch.


Drafted with Claude's assistance

  • Root cause was confirmed by reading parse_embedded and the two scanner paths (replace_delimiters with and without comma replacement) directly.
  • New unit tests were shown to fail on unfixed main and pass with the fix.
  • Every before/after value in the table comes from running the input through locally built cea.exe binaries (unfixed and fixed) and reading the .out files.
  • Bit-identical behavior for previously-correct inputs was checked by byte-comparing .out files from unmodified nasa/cea main and this branch for all legacy examples and samples/rp1311_examples.inp.

🤖 Generated with Claude Code

…307)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant