Skip to content

op2: read 584-byte PVT0 records as PARAM data and add RVMASS - #880

Merged
SteveDoyle2 merged 2 commits into
SteveDoyle2:mainfrom
Shaisolaris:fix-pvt0-584-rvmass
Oct 16, 2026
Merged

SteveDoyle2 merged 2 commits into
SteveDoyle2:mainfrom
Shaisolaris:fix-pvt0-584-rvmass

Conversation

@Shaisolaris

Copy link
Copy Markdown
Contributor

Fixes #875.

Two problems reading NX PARAM data from an OP2:

  1. _read_subtable_3_4 treats any 584-byte record as a table3 header. For PVT/PVT0/PVTS the table3 parser is _read_pvto_3, which raises, so an OP2 whose PARAM record happens to be 584 bytes long fails to read. This keeps the PARAM tables out of that check (PARAM_TABLES = {b'PVT', b'PVT0', b'PVTS'}), so the record goes to the table4 parser like any other PARAM record. I kept it to the PARAM tables rather than all of GEOM_TABLES to keep the change small.
  2. PARAM,RVMASS,YES is a string PARAM that was missing from STR_PARAMS_1, so _read_pvto_4_helper asserts (word=b'RVMASS') and the PARAM block is skipped. This adds RVMASS to STR_PARAMS_1.

Testing: I added TestOP2Params to test_op2_unit_tests.py:

  • test_pvt0_rvmass reads an RVMASS/AUTOSPC PARAM record. It fails on main with AssertionError: word=b'RVMASS' and passes with this change.
  • test_pvt0_record_of_length_584 runs _read_subtable_3_4 on a 584-byte PVT0 record with the record reads stubbed. On main the record goes to the table3 parser; with this change it goes to the table4 parser.

Both pass. I could not complete a full run of test_op2_unit_tests.py or test_op2_bugfixes.py locally within my time limit, so I am relying on CI for the rest.

A PVT/PVT0/PVTS (PARAM) data record that happens to be exactly 584 bytes
long was sent to the table3 parser, which for these tables is
_read_pvto_3 and raises. Keep PARAM tables out of the 584-byte table3
check so the record goes to the table4 parser.

PARAM,RVMASS,YES (NX) is a string PARAM that was missing from
STR_PARAMS_1, so reading it hit an assertion and the PARAM block was
skipped.

Fixes SteveDoyle2#875
SteveDoyle2
SteveDoyle2 previously approved these changes Oct 14, 2026
@codecov

codecov Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 27.58621% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.17%. Comparing base (ad407d2) to head (7495e43).

Files with missing lines Patch % Lines
pyNastran/op2/test/test_op2_unit_tests.py 22.22% 21 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #880      +/-   ##
==========================================
- Coverage   76.18%   76.17%   -0.01%     
==========================================
  Files        1093     1093              
  Lines      358978   359006      +28     
  Branches    37051    37051              
==========================================
+ Hits       273470   273477       +7     
- Misses      75834    75855      +21     
  Partials     9674     9674              
Files with missing lines Coverage Δ
pyNastran/op2/op2_interface/op2_reader.py 78.59% <100.00%> (+<0.01%) ⬆️
pyNastran/op2/op2_interface/op2_scalar.py 76.66% <ø> (ø)
pyNastran/op2/test/test_op2_unit_tests.py 93.93% <22.22%> (-0.88%) ⬇️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad407d2...7495e43. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@SteveDoyle2

Copy link
Copy Markdown
Owner

The tests were not run because a new class was created. They are not automatically discovered. New op2 test classes need to be added to pyNastran/op2/test/all_tests.py

@SteveDoyle2
SteveDoyle2 merged commit cfdac47 into SteveDoyle2:main Oct 16, 2026
2 of 21 checks passed
@SteveDoyle2

Copy link
Copy Markdown
Owner

thanks! I appreciate the PR!

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.

Two issues reading NX Nastran 2025 PARAM data from OP2 (PVT0 table misrouted + RVMASS not recognized)

2 participants