Fix parsing of Nastran small field reals in the modal structural solver - #2859
Open
gaoflow wants to merge 1 commit into
Open
Fix parsing of Nastran small field reals in the modal structural solver#2859gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
The mesh reader decodes the eight character fields of the GRID and CORD2R
entries positionally: nastran_float() rewrote every "-" into "e-" and then
dropped a leading "e" by looking at index 0, and __checkBlankField() only
recognised a blank field when it was exactly eight spaces long.
Both assumptions only hold when a field is left justified and fills all
eight columns. The Quick Reference Guide states that fields 2 through 9 do
not need to be either right or left justified, and Nastran trims trailing
blanks from the lines of the sorted bulk data echo, so:
- any negative coordinate that is not left flush raised ValueError, e.g.
" -5.2" became " e-5.2";
- a lower case "e" or a "D" exponent was never recognised, so "700.e-2"
became "700.ee-2" in any justification;
- a GRID entry without the optional CD field ends the echo line after X3,
leaving an empty slice that raised ValueError in int().
nastran_float() now normalises the field before interpreting it and inserts
the omitted exponent letter at the first sign after the mantissa sign, which
is what the reference readers do. It is defined at module level so it can be
tested; all twelve call sites use it unchanged.
6 tasks
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.
Proposed Changes
SU2_PY/SU2_Nastran/pysu2_nastran.pyreads the GRID and CORD2R entries of a Nastran sorted bulk data echo by slicing eight character fields and then interpreting each slice positionally. Two helpers do that interpretation, and both are only correct when the value happens to be left flush and to fill all eight columns:Neither assumption is guaranteed. The Quick Reference Guide, Format of Bulk Data Entries -> Small Field Format, says:
and on the same page:
So three families of legal input abort the FSI run before it starts:
nastran_float(" -5.2")builds" e-5.2"and raisesValueError. This does not need the E-less exponent form at all; a plain right justified negative coordinate is enough. pyNastran, for one, writes every small field with'%8s'/'%8d'/'%8.Nf', i.e. right justified.eandDexponents."700.e-2"becomes"700.ee-2"and".7D1"is never converted, in any justification. Both are accepted real formats (.1d-5is in the OptiStruct bulk data guidelines, and the QRG notes that "a double precision specification requires a 'D' type exponent").line[48:56]is''->int('')raises. In the tutorial mesh (Tutorials/multiphysics/unsteady_fsi_python/Ma01/modal.f06) CD is written explicitly, which is why this has not shown up; one column shorter and it is an empty slice.The three symptoms are one root cause, so the fix is at the two helpers rather than at the twelve GRID/CORD2R call sites, which are unchanged.
nastran_floatnormalises the field first and then inserts the omitted exponent letter at the first sign after the mantissa sign, which is what the established readers do;__checkBlankFieldtreats any whitespace-only or absent field as blank.nastran_floatmoved to module level so it can be tested — it was a closure inside__readNastranMesh.Verification
I enumerated the QRG spellings of a real crossed with sign and with left/right/centre justification in an eight column field: 108 fields, 49 of them rejected or miscomputed before, 0 after. The failures group as:
e/DexponentSU2_PY/SU2_Nastran/test_pysu2_nastran.pyis new: it drives that table throughnastran_float, and then builds the same little model (a CORD2R plus three GRIDs with negative coordinates, an E-less exponent, a blank CP and an omitted CD) in each justification and reads it through__readNastranMesh, asserting the parsed geometry does not depend on how the fields were written. 10 tests, all 10 fail ondevelopand pass here.There is no Python test harness in the repository at the moment, so this is plain
unittestwith no new dependency, run with:Happy to drop the file if you would rather not start a Python test directory here.
As a no-regression check I read the real tutorial
modal.f06(124 grid points, 41 negative coordinate values) withdevelopand with this branch: the parsed coordinates, IDs, CP, CD and marker sets are bit-for-bit identical.pre-commit runis clean (black 22.6.0). I did not build the C++ side; nothing outside this Python module is touched.Related Work
No related PR that I can find. #2313 is a different problem in the same file (page headers interrupting a SET1 continuation in the echo) and is not addressed here.
PR Checklist
pre-commit run --allto format old commits.