Bug Fixes for XML Parser in get_rule_mod

[akutuva21]

Pull Request: Bug Fixes for XML Parser in get_rule_mod

Summary

This PR addresses four critical parser bugs in the xmlparsers.py file's get_rule_mod method that were causing runtime errors and incorrect parsing behavior when processing BioNetGen XML files. The fixes ensure robust handling of missing attributes, type inconsistencies, and iteration bugs in the rule modification parser.

Note on branch history: While the bolt-performance-optimization branch contains 19+ commits with various experimental improvements, after merging with upstream main, only the parser bug fixes remain as net changes. Other experimental work (CI updates, performance optimizations) was superseded by upstream updates including PR #63.

Changes Made

Files Modified

• bionetgen/modelapi/xmlparsers.py - 16 lines modified (8 additions, 8 deletions)
• tests/test_get_rule_mod.py - New file with 125 lines of comprehensive test coverage

Bug Fixes in xmlparsers.py

1. Fixed Missing @DeleteMolecules Attribute Handling (Line 705)

Before:

dmvals = [op["@DeleteMolecules"] for op in del_op]

After:

dmvals = [op.get("@DeleteMolecules", "0") for op in del_op]

Issue: KeyError when @DeleteMolecules attribute was missing from Delete operations.
Fix: Use .get() with default value "0" to safely handle missing attributes.

2. Fixed all() Function Misuse (Line 708)

Before:

if all(dmvals) == 1:

After:

if all(val == "1" for val in dmvals):

Issue: all() returns a boolean, not an integer. Comparing True == 1 worked accidentally but was semantically incorrect.
Fix: Properly check if all values in the list equal "1".

3. Fixed Variable Reference Bug in MoveConnected Loop (Lines 734-737)

Before:

for mo in move_op:
    if mo["@moveConnected"] == "1":
        rule_mod.type = "MoveConnected"
        rule_mod.id.append(move_op["@id"])        # Wrong variable!
        rule_mod.source.append(move_op["@source"])  # Wrong variable!
        rule_mod.destination.append(move_op["@destination"])  # Wrong variable!
        rule_mod.flip.append(move_op["@flipOrientation"])  # Wrong variable!

After:

for mo in move_op:
    if mo["@moveConnected"] == "1":
        rule_mod.type = "MoveConnected"
        rule_mod.id.append(mo["@id"])              # Correct!
        rule_mod.source.append(mo["@source"])        # Correct!
        rule_mod.destination.append(mo["@destination"])  # Correct!
        rule_mod.flip.append(mo["@flipOrientation"])  # Correct!

Issue: Loop variable was mo but code was referencing move_op (the list, not the item). This would fail for lists or append incorrect data for single dictionaries.
Fix: Changed all references from move_op to mo to use the loop iteration variable.

4. Fixed TotalRate Type Handling and Missing Attribute (Lines 742-747)

Before:

if rate_type == "Function" and ratelaw["@totalrate"] == 1:
    # ...
    rule_mod.call = ratelaw["@totalrate"]

After:

if rate_type == "Function" and str(ratelaw.get("@totalrate", "0")) == "1":
    # ...
    rule_mod.call = ratelaw.get("@totalrate", "0")

Issues:

• KeyError when @totalrate attribute was missing
• Type comparison bug: @totalrate could be string "1" or integer 1 from XML parser

Fix:

• Use .get() with default "0" to handle missing attributes
• Convert to string for consistent comparison regardless of XML parser's type choice

Test Coverage

Added comprehensive unit tests in tests/test_get_rule_mod.py covering:

1. TotalRate handling:

   • Integer value (@totalrate: 1)
   • String value (@totalrate: "1")
   • Missing attribute
   • Zero value (@totalrate: "0")

2. DeleteMolecules handling:

   • Multiple Delete operations with attribute
   • Missing @DeleteMolecules attribute

3. MoveConnected handling:

   • Multiple ChangeCompartment operations (tests the loop fix)
   • Single ChangeCompartment operation
   • Correct extraction of id, source, destination, flip values

4. Precedence testing:

   • DeleteMolecules takes precedence over TotalRate when both present

All tests pass and verify that the parser no longer crashes on edge cases and handles both string and integer attribute values correctly.

Impact

These fixes prevent runtime crashes and ensure correct parsing of BioNetGen XML files with:

• Rules that have Delete operations without explicit @DeleteMolecules attributes
• Rules with TotalRate modifiers where the attribute might be int or string
• Rules with multiple MoveConnected operations
• Various edge cases in rule modification parsing

Testing

To run the new tests:

pytest tests/test_get_rule_mod.py -v

All existing tests continue to pass, confirming backward compatibility.

Branch History

The bolt-performance-optimization branch originally contained multiple experimental changes:

• Performance improvements (caching find_BNG_path, removing pkg_resources)
• GitHub Actions CI/CD updates and Python version compatibility fixes
• Code formatting with black
• libsbml attribute error fixes
• Removal of deprecated files (patch.py)

However, after merging upstream main (which included PR #63 with SBML compatibility improvements), most experimental changes were superseded by the upstream versions. Only the xmlparsers bug fixes remain as net changes from this branch.

Key Commits

• 065c6fd - ⚡ Bolt: [performance improvement] Cache find_BNG_path (superseded by merge)
• dd1a811 through 2706407 - CI/CD improvements (superseded by merge)
• acaa6e8 through bb65478 - Parser bug fixes (retained after merge)
• d011fac - Merge main into branch (resolved conflicts, kept parser fixes)

Checklist

• Bug fixes implemented
• Comprehensive unit tests added
• All tests passing
• Code formatted with black
• No breaking changes to existing functionality
• Documentation (via tests) demonstrates correct behavior

---

Branch: bolt-performance-optimization-6035337824748401789
Base: main
Author: @akutuva21