Jules changes - #83
Merged
Merged
Conversation
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Removed a block of dead code in `src/NFfunction/muParser/muParser.cpp` that was commented out since 2008 because it lacked locale support. This improves code health by removing obsolete code. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Adds missing unit tests for Compartment::isInside method covering null pointer cases, identity checks, and various parent/child/sibling hierarchy relationships. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Removed extensive commented out blocks of code in logClassSelector.cpp and directSelector.cpp that pertained to numerical errors and dead logic. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Changed `it++` to `++it` in the `reportedSpecies` map iteration loop within `System::saveSpecies`. This is a standard micro-optimization that avoids the creation of a temporary iterator copy during each loop iteration. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
This commit modifies the `GlobalFunction` constructor in `NFfunction.hh` and `function.cpp` to take `name` and `funcExpression` arguments as `const std::string&` instead of pass-by-value `std::string`. This avoids unnecessary string copies when creating functions, optimizing performance. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Removed commented-out loop code and associated orphaned comment in `System::getGlobalFunctionByName` in `src/NFcore/system.cpp` to improve readability and code health. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Removed dead code block of alternate compare logic that was commented out in `src/NFreactions/reactions/reaction.cpp` to improve code readability and maintainability. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Merged two consecutive loops in `parseFuncXML.cpp` that iterated over `refNames.size()` into a single loop. Additionally, cached the result of `refTypes.at(rn)` to a `const string&` variable to prevent multiple bounds-checked vector access lookups per iteration. This safely optimizes the function XML parsing logic while preserving identical functionality, as an early termination (`exit(1)`) on an invalid "Observable" reference will immediately halt the process, ignoring the partially populated `functionsCalled` vector state. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Changed `setCtrName(string name)` to `setCtrName(const string& name)` in `CompositeFunction` and `GlobalFunction` classes within `src/NFfunction` to avoid unnecessary string passing by value and string allocations. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Removes the legacy comment "// Fix for [ 1663758 ] Failure to report error on bad XML" from src/NFinput/TinyXML/tinyxmlparser.cpp as it was merely referencing a fix already merged for an old issue tracker ID. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Replaced sequential snprintf calls and manual offset management with std::ostringstream to prevent potential buffer overflows. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Replaced `explicitOutputTimes.at(i)` with `explicitOutputTimes[i]` in the explicit output times loop in `src/NFsim.cpp`. Since the loop bounds are exactly constrained from `0` to `explicitOutputTimes.size()`, the bounds checking performed by `.at()` is redundant and adds unnecessary overhead. Direct array access ensures maximum performance while remaining completely safe. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Changes `GlobalFunction::enableFileDependency` and `CompositeFunction::enableFileDependency` and their respective declarations in `NFfunction.hh` to use `const string&` for string parameters instead of pass-by-value `string`. This avoids unnecessary copying of string arguments during function calls. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Extracted the large block of TFUN function processing from `NFinput::initFunctions` into a dedicated helper function `processTfunFunction`. This vastly improves the readability and maintainability of the parser. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Clean up stale and misleading comments about a "terrifying little bug" in the Microsoft STL implementation that was actually an aliasing issue, and remove the commented-out StringToBuffer workarounds. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Replaced dirty dummy function wrappers (`rand`, `strlen`, `strncmp`) within `namespace std` in `muParserFixes.h` with standard C++ `using` declarations to improve code health and maintainability. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
…146039226902 🧹 Remove Rob Laveaux bug fix comment
…6904684217420752 ⚡ Optimize map passing in configuration methods
…pare-3290434787948349438 🧹 Refactor TemplateMolecule::compare to improve readability
…n-1153124635402002888 🧹 [code health] Refactor tfun_interpolate_value to use standard algorithms
…440929702096 ⚡ Optimize usedComponentNames search in NFinput
…11570811 🧹 [code health improvement] Fix fread logic for line normalization in TinyXML
…-13299254897190932162 ⚡ Optimize MoleculeType observable additions
This commit introduces a new test suite under src/NFtest/templateMolecule to specifically target the `TemplateMolecule::printDetails` functionality. During the creation of this test suite, a segmentation fault was discovered where `mappedTm` could be unconditionally dereferenced despite being uninitialized (NULL). This commit addresses that bug by wrapping the dereference in a NULL check, thereby increasing the robustness of the system alongside expanding its test coverage. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
…2769411623 🧪 Missing test for TemplateMolecule::printDetails
This commit replaces unsafe C-style string conversions `atoi` and `atof` with the safer standard alternatives `strtol` and `strtod` in TinyXML. 🎯 **What:** Replaced `atoi` with `static_cast<int>(strtol(...))` and `atof` with `strtod(...)` in `TiXmlElement::Attribute` and `TiXmlAttribute::IntValue`/`DoubleValue` methods inside `src/NFinput/TinyXML/tinyxml.cpp`.⚠️ **Risk:** `atoi` and `atof` invoke undefined behavior upon integer or floating-point overflow. This can be exploited to cause crashes or unpredictable application states if attacker-controlled XML input contains maliciously crafted numeric attributes exceeding data type boundaries. 🛡️ **Solution:** `strtol` and `strtod` provide well-defined fallback behavior on overflow (returning `LONG_MAX`/`LONG_MIN` or `HUGE_VAL`) and do not invoke undefined behavior, ensuring application stability without breaking legacy parsing behavior. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Removed unused `DEBUG_MESSAGE` macros, conditionally executed debug outputs, and commented-out `cout` lines in `reactantTree.cpp` and `DORreaction.cpp`. Cleans up dead code and conditional debug blocks that cluttered the codebase, improving overall readability without altering functional behavior. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
…s-3914749750055738879 🧹 Remove commented-out debug blocks in ReactantTree and DORreaction
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
…8787329845084892225 🧪 Add test for Compartment::printDetails
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
…ionRules-5040471865167564296 🧹 [code health improvement] Refactor initReactionRules
Replaced unnecessary string pass-by-value arguments with pass-by-const-reference in enableFileDependency, enableInlineDependency, setInterpolationMethod, and setCtrName in the GlobalFunction and CompositeFunction classes. This prevents redundant string copies during function setup and parsing. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
⚡ Optimize string passing in file dependency functions
…73359978726 🔒 Replace atoi/atof with safer alternatives in TinyXML
This commit adds a new unit test suite for the `ReactionClass::fire` method to improve codebase coverage and testing reliability. The test explicitly verifies the behavior of `fire` both with and without event tracking by asserting changes to `fireCounter`. Included is the registration of the new test suite in CMake build files and NFsim execution pathways. All C++ and Python validation test suites pass. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
…4067789985157 🧪 Add tests for ReactionClass::fire
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>
…38631055 🧪 Add test for MoleculeType::printDetails
…ng-7210120735045215432 ⚡ Optimize map passing in argument handling
- Fix model r16 validation: add per-iteration reset of ssaDiff/nfDiff so each seed is evaluated independently instead of cumulatively - Add model-specific tolerance (0.5 for r16) via targetedTests config - Update actions: checkout@v3->v4, setup-python@v2->v5, cache@v3->v4 - Replace deprecated unittest.makeSuite with TestLoader.loadTestsFromTestCase
fix: resolve CI validation failures and update deprecated actions
…nd delete .jules/
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
1 participant
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.
Code cleanup, performance, and test coverage pass
Branch:
akutuva21:master→masterSummary
A batch of 47 commits focused on dead-code removal, micro-performance optimizations, added unit-test coverage, and CI maintenance. No functional/behavioral changes to simulation output are intended — these are health, performance, and tooling changes plus new tests.
Scope: ~50 files touched across
NFcore,NFfunction,NFinput(incl. TinyXML/muParser),NFreactions,NFscheduler, andNFtest, plus build files and CI config.Highlights
const-reference, replaced bounds-checked.at()with direct indexing, converted post- to pre-increment on iterators, and tightened lookups (e.g.unordered_setfor observable additions, optimized function-reference and component-name lookups).Compartment::isInside,Compartment::printDetails,Molecule::printDetails,Complex::printDetails,TemplateMolecule::printDetails,MoleculeType::printDetails, and a suite forReactionClass::fire(including a related segfault fix).atoi/atofwith safer parsing in TinyXML; fixedfreadline-normalization logic; fixed buffer manipulation injob2str; fixed a missing parenthesis in a test error path.initFunctions,initStartSpecies,initReactionRules) and simplifiedTemplateMolecule::compare,tfun_*helpers, and namespace handling.Commits
Testing
New unit tests added under
src/NFtest/. CI updated in.github/workflows/main-testing.yml;validate/validate.pyadjusted.Notes
No public API or simulation-behavior changes intended. Build files (
CMakeLists.txt,CMakeLists.x86.txt) updated to pick up new test sources.