Jacobian sparsity filter in networks/rhs.H does not exclude intermediate reactions as intended
Summary
RHS::is_jacobian_term_used() attempts to exclude intermediate reactions (those with any __extra__ species index > NumSpec) when deciding if a Jacobian entry is needed. The implementation uses || across <= NumSpec tests, which is almost always true, so intermediate reactions are not filtered out.
Severity
Medium (primarily correctness-of-pruning/performance; can increase unnecessary Jacobian work and obscure intended sparsity behavior)
Affected Code
networks/rhs.H
- around lines 235-240 and 256-260
Why This Is a Bug
The comments state that reactions with extra species should be excluded. That requires all participating species IDs to be within NumSpec (&& logic). The current || logic admits any reaction with at least one in-network species, including intermediate reactions.
Proposed Patch
--- a/networks/rhs.H
+++ b/networks/rhs.H
@@
- if (data.species_A <= NumSpec ||
- data.species_B <= NumSpec ||
- data.species_C <= NumSpec ||
- data.species_D <= NumSpec ||
- data.species_E <= NumSpec ||
- data.species_F <= NumSpec) {
+ if (data.species_A <= NumSpec &&
+ data.species_B <= NumSpec &&
+ data.species_C <= NumSpec &&
+ data.species_D <= NumSpec &&
+ data.species_E <= NumSpec &&
+ data.species_F <= NumSpec) {
is_spec_1_used = 1;
}
@@
- if (data.species_A <= NumSpec ||
- data.species_B <= NumSpec ||
- data.species_C <= NumSpec ||
- data.species_D <= NumSpec ||
- data.species_E <= NumSpec ||
- data.species_F <= NumSpec) {
+ if (data.species_A <= NumSpec &&
+ data.species_B <= NumSpec &&
+ data.species_C <= NumSpec &&
+ data.species_D <= NumSpec &&
+ data.species_E <= NumSpec &&
+ data.species_F <= NumSpec) {
is_spec_2_used = 1;
}
Validation
- Compare Jacobian nonzero pattern before/after for a network with intermediate reactions.
- Confirm species evolution and energy RHS/Jacobian numerics are unchanged (only pruning logic should change).
Jacobian sparsity filter in
networks/rhs.Hdoes not exclude intermediate reactions as intendedSummary
RHS::is_jacobian_term_used()attempts to exclude intermediate reactions (those with any__extra__species index >NumSpec) when deciding if a Jacobian entry is needed. The implementation uses||across<= NumSpectests, which is almost always true, so intermediate reactions are not filtered out.Severity
Medium (primarily correctness-of-pruning/performance; can increase unnecessary Jacobian work and obscure intended sparsity behavior)
Affected Code
networks/rhs.HWhy This Is a Bug
The comments state that reactions with extra species should be excluded. That requires all participating species IDs to be within
NumSpec(&&logic). The current||logic admits any reaction with at least one in-network species, including intermediate reactions.Proposed Patch
Validation