Skip to content

Commit 37707bf

Browse files
binggao1230binggao1230
authored
Fix parsing of Nastran small field reals in the modal structural solver (#2859)
## Proposed Changes `SU2_PY/SU2_Nastran/pysu2_nastran.py` reads 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: ```python def nastran_float(s): if s.find("E") == -1: s = s.replace("-", "e-") s = s.replace("+", "e+") if s[0] == "e": # only removes the spurious "e" if it lands on index 0 s = s[1:] return float(s) def __checkBlankField(self, string): if string == " " * 8: # only recognises a blank field exactly eight characters wide return int(0) return int(string) ``` Neither assumption is guaranteed. The Quick Reference Guide, *Format of Bulk Data Entries -> Small Field Format*, says: > Fields 1 and 10 must be left justified. > Fields 2 through 9 do not need to be either right or left justified, although aligning the data fields is good practice. and on the same page: > Real numbers may be entered in a variety of ways. For example, the following are all acceptable versions of the real number seven: > `7.0 .7E1 0.7+1 .70+1 7.E+0 70.-1` So three families of legal input abort the FSI run before it starts: 1. **Any negative real that is not left flush.** `nastran_float(" -5.2")` builds `" e-5.2"` and raises `ValueError`. 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. 2. **Lower case `e` and `D` exponents.** `"700.e-2"` becomes `"700.ee-2"` and `".7D1"` is never converted, in *any* justification. Both are accepted real formats (`.1d-5` is in the OptiStruct bulk data guidelines, and the QRG notes that "a double precision specification requires a 'D' type exponent"). 3. **A GRID entry without the optional CD field.** Nastran trims the trailing blanks of each echo line, so the entry ends after X3 and `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_float` normalises 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; `__checkBlankField` treats any whitespace-only or absent field as blank. `nastran_float` moved 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: | family | left | right | centre | |---|---|---|---| | QRG spellings of seven | 0 | 8 | 8 | | lower case `e` / `D` exponent | 9 | 13 | 11 | `SU2_PY/SU2_Nastran/test_pysu2_nastran.py` is new: it drives that table through `nastran_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 on `develop` and pass here. There is no Python test harness in the repository at the moment, so this is plain `unittest` with no new dependency, run with: ``` python -m unittest discover -s SU2_PY/SU2_Nastran ``` 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) with `develop` and with this branch: the parsed coordinates, IDs, CP, CD and marker sets are bit-for-bit identical. `pre-commit run` is 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 - [x] I am submitting my contribution to the develop branch. - [x] My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson). Nothing compiled changes. - [x] My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/). - [x] I used the pre-commit hook to prevent dirty commits and used `pre-commit run --all` to format old commits. - [x] I have added a test case that demonstrates my contribution, if necessary. - [ ] I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.
1 parent b12aa0c commit 37707bf

4 files changed

Lines changed: 234 additions & 10 deletions

File tree

‎SU2_PY/SU2_Nastran/pysu2_nastran.py‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,30 @@
3333
import scipy.linalg as linalg
3434
from math import *
3535

36+
# ----------------------------------------------------------------------
37+
# Bulk data field parsing
38+
# ----------------------------------------------------------------------
39+
40+
41+
def nastran_float(s):
42+
"""
43+
This method converts one small field of a bulk data entry into a float.
44+
45+
Fields 2 through 9 do not need to be either right or left justified, and
46+
the exponent may be written with an "E", with a "D", or with no letter at
47+
all: "7.0", ".7E1", "0.7+1", ".70+1", "7.E+0" and "70.-1" all mean seven.
48+
"""
49+
50+
s = s.strip().upper().replace("D", "E")
51+
if "E" not in s:
52+
# The exponent letter is omitted, so the exponent starts at the first
53+
# sign which is not the sign of the mantissa.
54+
signs = [i for i in (s.find("+", 1), s.find("-", 1)) if i > 0]
55+
if signs:
56+
s = s[: min(signs)] + "E" + s[min(signs) :]
57+
return float(s)
58+
59+
3660
# ----------------------------------------------------------------------
3761
# Config class
3862
# ----------------------------------------------------------------------
@@ -426,14 +450,6 @@ def __readNastranMesh(self):
426450
This method reads the nastran 3D mesh.
427451
"""
428452

429-
def nastran_float(s):
430-
if s.find("E") == -1:
431-
s = s.replace("-", "e-")
432-
s = s.replace("+", "e+")
433-
if s[0] == "e":
434-
s = s[1:]
435-
return float(s)
436-
437453
self.nMarker = 0
438454
self.nPoint = 0
439455
self.nRefSys = 0
@@ -573,7 +589,8 @@ def __checkBlankField(self, string):
573589
This method considers that Nastran apply 0 when the reference system is not specified
574590
"""
575591

576-
if string == " " * 8:
592+
string = string.strip()
593+
if not string:
577594
return int(0)
578595
return int(string)
579596

‎SU2_PY/SU2_Nastran/test_pysu2_nastran.py‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
#!/usr/bin/env python
2+
3+
## \file test_pysu2_nastran.py
4+
# \brief Tests for the bulk data parsing of the Nastran structural solver.
5+
# \version 8.5.0 "Harrier"
6+
#
7+
# SU2 Project Website: https://su2code.github.io
8+
#
9+
# The SU2 Project is maintained by the SU2 Foundation
10+
# (http://su2foundation.org)
11+
#
12+
# Copyright 2012-2026, SU2 Contributors (cf. AUTHORS.md)
13+
#
14+
# SU2 is free software; you can redistribute it and/or
15+
# modify it under the terms of the GNU Lesser General Public
16+
# License as published by the Free Software Foundation; either
17+
# version 2.1 of the License, or (at your option) any later version.
18+
#
19+
# SU2 is distributed in the hope that it will be useful,
20+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
21+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
22+
# Lesser General Public License for more details.
23+
#
24+
# You should have received a copy of the GNU Lesser General Public
25+
# License along with SU2. If not, see <http://www.gnu.org/licenses/>.
26+
27+
import os
28+
import sys
29+
import tempfile
30+
import unittest
31+
32+
import numpy as np
33+
34+
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
35+
36+
from pysu2_nastran import Solver, nastran_float
37+
38+
# All the spellings of the real number seven listed in the Quick Reference
39+
# Guide, "Format of Bulk Data Entries", plus the "D" exponent and the lower
40+
# case forms accepted by the bulk data readers.
41+
SEVEN = [
42+
"7.0",
43+
".7E1",
44+
"0.7+1",
45+
".70+1",
46+
"7.E+0",
47+
"70.-1",
48+
".7e1",
49+
"700.e-2",
50+
"7.0D0",
51+
".7d1",
52+
"70.-01",
53+
"7000.-3",
54+
]
55+
56+
57+
def justify(value, style):
58+
"""Places a value in an eight character field, as Nastran allows."""
59+
60+
if style == "left":
61+
return value.ljust(8)
62+
if style == "right":
63+
return value.rjust(8)
64+
return value.center(8)
65+
66+
67+
def card(fields, style, prefix=30):
68+
"""Builds one line of a small field bulk data echo."""
69+
70+
line = fields[0].ljust(8)
71+
for field in fields[1:]:
72+
line += justify(field, style)
73+
return " " * prefix + line.rstrip() + "\n"
74+
75+
76+
class TestNastranFloat(unittest.TestCase):
77+
def test_spellings_of_seven(self):
78+
for value in SEVEN:
79+
for style in ("left", "right", "centre"):
80+
field = justify(value, style)
81+
self.assertAlmostEqual(nastran_float(field), 7.0, msg=repr(field))
82+
83+
def test_negative_spellings_of_seven(self):
84+
for value in SEVEN:
85+
for style in ("left", "right", "centre"):
86+
field = justify("-" + value, style)
87+
self.assertAlmostEqual(nastran_float(field), -7.0, msg=repr(field))
88+
89+
def test_leading_plus_is_not_an_exponent(self):
90+
for style in ("left", "right", "centre"):
91+
self.assertAlmostEqual(nastran_float(justify("+7.0", style)), 7.0)
92+
93+
def test_omitted_exponent_letter(self):
94+
self.assertAlmostEqual(nastran_float("1.23-5"), 1.23e-5)
95+
self.assertAlmostEqual(nastran_float(" -1.23-5"), -1.23e-5)
96+
self.assertAlmostEqual(nastran_float("-1.23+5"), -1.23e5)
97+
self.assertAlmostEqual(nastran_float(" 1+5"), 1.0e5)
98+
99+
def test_invalid_field_is_rejected(self):
100+
for field in ("", " ", "abc", "1.2.3"):
101+
with self.assertRaises(ValueError):
102+
nastran_float(field)
103+
104+
105+
class TestReadNastranMesh(unittest.TestCase):
106+
"""
107+
Reads the same model written with different, equally valid, spellings.
108+
The parsed geometry must not depend on how the fields were written.
109+
"""
110+
111+
def mesh(self, style, omit_optional_fields=False):
112+
cd = [] if omit_optional_fields else ["0"]
113+
lines = [
114+
card(
115+
["CORD2R", "1", "0", "-1.5", "-2.5", "-3.5", "-1.5", "-2.5", "-0.5"],
116+
style,
117+
),
118+
card(["+", "0.5", "-2.5", "-3.5"], style),
119+
card(["GRID", "1", "0", "-1.25", "-2.5", "3.75"] + cd, style),
120+
# A blank CP field means the basic coordinate system.
121+
card(["GRID", "2", "", "-1.5-2", "2.5-2", "-3.5+1"] + cd, style),
122+
card(["GRID", "3", "1", "1.0", "2.0", "-3.0"] + cd, style),
123+
card(["SET1", "1", "1", "2", "3"], style),
124+
]
125+
handle, path = tempfile.mkstemp(suffix=".f06")
126+
with os.fdopen(handle, "w") as mesh_file:
127+
mesh_file.writelines(lines)
128+
self.addCleanup(os.remove, path)
129+
return path
130+
131+
def read(self, style, omit_optional_fields=False):
132+
solver = Solver.__new__(Solver)
133+
solver.Mesh_file = self.mesh(style, omit_optional_fields)
134+
solver.FSI_marker = "1"
135+
solver.node = []
136+
solver.markers = {}
137+
solver.refsystems = []
138+
solver._Solver__readNastranMesh()
139+
return solver
140+
141+
def coordinates(self, solver):
142+
return np.array([point.GetCoord0().ravel() for point in solver.node])
143+
144+
def test_justification_does_not_change_the_model(self):
145+
reference = self.coordinates(self.read("left"))
146+
self.assertEqual(reference.shape, (3, 3))
147+
for style in ("right", "centre"):
148+
np.testing.assert_allclose(self.coordinates(self.read(style)), reference)
149+
150+
def test_coordinates_are_read_correctly(self):
151+
for style in ("left", "right", "centre"):
152+
coordinates = self.coordinates(self.read(style))
153+
# Point 1 is given in the basic system.
154+
np.testing.assert_allclose(coordinates[0], [-1.25, -2.5, 3.75])
155+
# Point 2 uses the omitted exponent letter.
156+
np.testing.assert_allclose(coordinates[1], [-0.015, 0.025, -35.0])
157+
# Point 3 is given in the reference system defined by the CORD2R,
158+
# whose origin is (-1.5, -2.5, -3.5) and whose x axis points to
159+
# (0.5, -2.5, -3.5), i.e. the basic x axis.
160+
np.testing.assert_allclose(coordinates[2], [-0.5, -0.5, -6.5])
161+
162+
def test_blank_reference_system_field_is_the_basic_system(self):
163+
for style in ("left", "right", "centre"):
164+
solver = self.read(style)
165+
self.assertEqual([point.GetCP() for point in solver.node], [0, 0, 1])
166+
167+
def test_optional_trailing_field_may_be_missing(self):
168+
for style in ("left", "right", "centre"):
169+
reference = self.coordinates(self.read(style))
170+
without_cd = self.coordinates(self.read(style, omit_optional_fields=True))
171+
np.testing.assert_allclose(without_cd, reference)
172+
173+
def test_reference_system_is_read_correctly(self):
174+
for style in ("left", "right", "centre"):
175+
solver = self.read(style)
176+
self.assertEqual(len(solver.refsystems), 1)
177+
system = solver.refsystems[0]
178+
self.assertEqual(system.GetCID(), 1)
179+
np.testing.assert_allclose(system.GetOrigin().ravel(), [-1.5, -2.5, -3.5])
180+
np.testing.assert_allclose(system.GetRotMatrix(), np.eye(3), atol=1e-12)
181+
182+
183+
if __name__ == "__main__":
184+
unittest.main()

‎SU2_PY/meson.build‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ install_data(['FSI_tools/__init__.py',
6868
install_dir: join_paths(get_option('bindir'), 'FSI_tools'))
6969

7070
install_data(['SU2_Nastran/__init__.py',
71-
'SU2_Nastran/pysu2_nastran.py'],
71+
'SU2_Nastran/pysu2_nastran.py',
72+
'SU2_Nastran/test_pysu2_nastran.py'],
7273
install_dir: join_paths(get_option('bindir'), 'SU2_Nastran'))
7374

7475
install_subdir(['../externals/FADO'], install_dir: get_option('bindir'))

‎TestCases/serial_regression.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
# License along with SU2. If not, see <http://www.gnu.org/licenses/>.
2727

2828
from __future__ import print_function, division, absolute_import
29+
import os
30+
import subprocess
2931
import sys
3032
from TestCase import TestCase
3133
from TestCase import parse_args
@@ -1257,6 +1259,26 @@ def main():
12571259

12581260
pass_list = [ test.run_test(args.tsan, args.asan) for test in test_list ]
12591261

1262+
# Nastran bulk data parser unit tests
1263+
nastran_parser = TestCase('pysu2_nastran')
1264+
# The CI container runs this script from a copied tests/TestCases tree, so
1265+
# the repo-relative path does not exist there; use the installed copy that
1266+
# SU2_RUN points to and fall back to the source tree for local runs.
1267+
nastran_test = os.path.join(
1268+
os.environ.get('SU2_RUN', ''),
1269+
'SU2_Nastran',
1270+
'test_pysu2_nastran.py',
1271+
)
1272+
if not os.path.isfile(nastran_test):
1273+
nastran_test = os.path.join(
1274+
os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
1275+
'SU2_PY',
1276+
'SU2_Nastran',
1277+
'test_pysu2_nastran.py',
1278+
)
1279+
pass_list.append(subprocess.call([sys.executable, nastran_test]) == 0)
1280+
test_list.append(nastran_parser)
1281+
12601282

12611283
######################################
12621284
### RUN SU2_GEO TESTS ###

0 commit comments

Comments
 (0)