Skip to content

fix(orca): implement NMD parsing, guard trsh against None - #1026

Open
Mia1Gohary wants to merge 3 commits into
mainfrom
orca-nmd-fix
Open

fix(orca): implement NMD parsing, guard trsh against None#1026
Mia1Gohary wants to merge 3 commits into
mainfrom
orca-nmd-fix

Conversation

@Mia1Gohary

Copy link
Copy Markdown

Orca's parse_normal_mode_displacement() was a stub returning (None, None). trsh_negative_freq() then called len(None) on any species with a genuine imaginary frequency, crashing the whole ARC process instead of nudging the geometry and re-optimizing.

Implement real parsing of Orca's VIBRATIONAL FREQUENCIES / NORMAL MODES blocks, and add a defensive None check in trsh.py so any adapter lacking this capability degrades gracefully instead of crashing.

Validated against CR_1's r1 freq job on zeus: correctly identified and parsed the -33.01 cm^-1 imaginary mode; live run confirmed freq troubleshooting completes (freq: True) where it previously crashed.

@Mia1Gohary
Mia1Gohary requested a review from alongd August 26, 2026 10:07
@github-actions github-actions Bot added the Module: trsh Troubleshooting label Aug 26, 2026
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.54%. Comparing base (59ac3f0) to head (c288059).
⚠️ Report is 171 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1026      +/-   ##
==========================================
+ Coverage   63.25%   64.54%   +1.28%     
==========================================
  Files         114      119       +5     
  Lines       38274    39886    +1612     
  Branches    10013    10326     +313     
==========================================
+ Hits        24212    25744    +1532     
- Misses      11158    11160       +2     
- Partials     2904     2982      +78     
Flag Coverage Δ
functionaltests 64.54% <ø> (+1.28%) ⬆️
unittests 64.54% <ø> (+1.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

@Mia1Gohary
Mia1Gohary force-pushed the orca-nmd-fix branch 2 times, most recently from 778878f to a77d920 Compare August 26, 2026 15:13
Orca's parse_normal_mode_displacement() was a stub returning (None, None).
trsh_negative_freq() then called len(None) on any species with a genuine
imaginary frequency, crashing the whole ARC process instead of nudging the
geometry and re-optimizing.

Implement real parsing of Orca's VIBRATIONAL FREQUENCIES / NORMAL MODES
blocks, and add a defensive None check in trsh.py so any adapter lacking
this capability degrades gracefully instead of crashing.

Validated against CR_1's r1 freq job on zeus: correctly identified and
parsed the -33.01 cm^-1 imaginary mode; live run confirmed freq
troubleshooting completes (freq: True) where it previously crashed.
Comment thread arc/parser/adapters/orca.py Outdated
Comment thread arc/parser/parser_test.py
@calvinp0

Copy link
Copy Markdown
Member

It's looking good @Mia1Gohary , just two comments for fixing!

  Reuse the Orca frequency parser when reading normal mode
  displacements, while retaining zero.
  Move the n-cetane checks into a dedicated test and verify that
  frequencies remain aligned with Cartesian displacement vectors.
@Mia1Gohary
Mia1Gohary requested a review from calvinp0 August 27, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module: trsh Troubleshooting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants