Repository navigation
Conversation
Contributor
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
akutuva21
force-pushed
the
jules-17678800431109127773-3cdc0d74
branch
from
September 14, 2026 14:57
34abbf1 to
7482f78
Compare
akutuva21
changed the title
akutuva21
marked this pull request as ready for review
September 14, 2026 21:53
Reverts benchmarks/README.md Atomizer copy benchmark section and removes
benchmarks/atomizer_copy_benchmark.py per maintainer preference. Core
optimization in python/bionetgen/atomizer/{modern/structures.py,utils/smallStructures.py,utils/structures.py} retained.
On main, Python 3.9 arm64 macOS gets a x86_64-only python-libsbml
wheel (dlopen: have 'x86_64', need 'arm64e'). Because
bionetgen.atomizer.__init__.py eagerly imported
atomizeTool -> libsbml2bngl -> libsbml at top-level, any import of
bionetgen.atomizer.* (even pure-Python helpers) failed, turning
21 pure tests into collection errors:
ImportError: dlopen(..._libsbml.cpython-39-darwin.so)
bionetgen/atomizer/__init__.py:1 -> atomizeTool.py:1 -> libsbml2bngl.py:11
Fix by making the SBML stack optional:
- atomizer/__init__.py: expose AtomizeTool lazily via PEP 562
__getattr__, so importing bionetgen.atomizer or
bionetgen.atomizer.modern/utils/writer no longer triggers libsbml.
- atomizeTool.py: lazy import libsbml2bngl, guard checkConfig/run
with _require_ls2b() that also checks internal libsbml.
- libsbml2bngl.py/sbml2bngl.py/sbml2json.py: wrap top-level
import libsbml in try/except, provide _require_libsbml(),
keep pure helpers (factorial/comb) importable.
- detectOntology.py/analyzeRDF.py: catch ImportError (not just
ModuleNotFoundError) so levenshtein etc. stay importable.
- tests: atomize tests now pytest.importorskip('libsbml',
exc_type=ImportError) so they skip (not fail) when the wheel is
missing/broken.
With broken libsbml, pytest collection now succeeds (354 collected)
and pure tests pass; SBML tests skip. With working libsbml,
behavior is unchanged.
* Replaced deepcopy with list comprehensions in atomizer structures to address performance bottleneck (from earlier plan). * Fixed NFIR cache fingerprint mismatch in C++ by correcting hashing vs caching order of coordinate_parameterized and default_rate fields. * Addressed `test_energy_validation_harness` failure by explicitly using setup python environments rather than system python via CMake. * Repaired ASan allocator mismatch failures by implementing allocation_probe disablement in memory tests and fixing CMake files. * Applied required format corrections via black and ruff fixing the github workflows formatting checks. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
* Replaced deepcopy with list comprehensions in atomizer structures to address performance bottleneck (from earlier plan). * Fixed NFIR cache fingerprint mismatch in C++ by correcting hashing vs caching order of coordinate_parameterized and default_rate fields. * Addressed `test_energy_validation_harness` failure by explicitly using setup python environments rather than system python via CMake. * Repaired ASan allocator mismatch failures by implementing allocation_probe disablement in memory tests and fixing CMake files. * Applied required format corrections via black and ruff fixing the github workflows formatting checks. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
* Replaced deepcopy with list comprehensions in atomizer structures to address performance bottleneck (from earlier plan). * Fixed NFIR cache fingerprint mismatch in C++ by correcting hashing vs caching order of coordinate_parameterized and default_rate fields. * Addressed `test_energy_validation_harness` failure by explicitly using setup python environments rather than system python via CMake. * Repaired ASan allocator mismatch failures by implementing allocation_probe disablement in memory tests and fixing CMake files. * Applied required format corrections via black and ruff fixing the github workflows formatting checks. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| bonds_.resize((slotIndex + 1u) * static_cast<std::size_t>(bond_slots_)); | ||
| compartments_.resize(slotIndex + 1u, 0); | ||
| states_.resize((static_cast<std::size_t>(slot) + 1u) * static_cast<std::size_t>(state_words_), 0); | ||
| bonds_.resize((slot + 1) * bond_slots_); |
| if (!appendDigit(static_cast<unsigned long>(*q - 'A' + 10), 16)) return 0; | ||
| } | ||
| if ( *q >= '0' && *q <= '9' ) | ||
| ucs += mult * (*q - '0'); |
| if ( *q >= '0' && *q <= '9' ) | ||
| ucs += mult * (*q - '0'); | ||
| else if ( *q >= 'a' && *q <= 'f' ) | ||
| ucs += mult * (*q - 'a' + 10); |
| else if ( *q >= 'a' && *q <= 'f' ) | ||
| ucs += mult * (*q - 'a' + 10); | ||
| else if ( *q >= 'A' && *q <= 'F' ) | ||
| ucs += mult * (*q - 'A' + 10 ); |
| if (!appendDigit(static_cast<unsigned long>(*q - '0'), 10)) return 0; | ||
| } | ||
| if ( *q >= '0' && *q <= '9' ) | ||
| ucs += mult * (*q - '0'); |
Member
Author
|
Superseded by merged PR #23. |
Contributor
Understood. Acknowledging that this work is superseded and stopping work on this task. |
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
2 participants
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.
Summary
deepcopywith list copies for primitive Atomizer bonds, states, and bond metadata while retaining recursive molecule/species copying.Component.copy()bonds, states, and active state in independent lists.deepcopybaseline and reports timing and traced allocations.Validation
PYTHONPATH=python python -m pytest tests/python/test_atomizer_copy_contracts.py tests/python/test_modern_atomizer_structures.py tests/python/test_smallStructures.py -q— 18 passed.PYTHONPATH=python python benchmarks/atomizer_copy_benchmark.py --iterations 500 --repeats 5— passed with copy-isolation assertions.The local full Python pass reached 280 passed and 57 skipped; 13 compatibility/CLI tests require the compiled BNG3 backend, which is not built in this clean source checkout. Hosted CI has been restarted for this exact head.