Skip to content

Port improved comp flattening from eikept/jsbml and run tests #277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dyrpsf wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Port improved comp flattening from eikept/jsbml and run tests #277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7be7ef
Copy label and associatedSpecies in GeneProduct copy constructor
dyrpsf Jan 13, 2026
0ac6e59
Handle rateOf csymbol in OverdeterminationValidator for L3V2
dyrpsf Jan 14, 2026
924bb78
Port improved comp flattening from eikept/jsbml fork
dyrpsf Jan 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 83 additions & 30 deletions core/src/org/sbml/jsbml/validator/OverdeterminationValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,46 @@ public Map<SBase, SBase> getMatching() {
return matching;
}

/**
* Resolve an identifier from a MathML {@link ASTNode} to the
* corresponding {@link SBase} object in the model.
*
* This is in particular needed for SBML Level&nbsp;3 Version&nbsp;2
* {@code rateOf} csymbols, where the {@link ASTNode#getVariable()}
* may not yet be initialized.
*
* @param id the SBML id of the object
* @return the corresponding {@link SBase}, or {@code null} if none
* can be found.
*/
private SBase resolveSBaseFromId(String id) {
if (model == null || id == null) {
return null;
}

Species species = model.getSpecies(id);
if (species != null) {
return species;
}

Compartment compartment = model.getCompartment(id);
if (compartment != null) {
return compartment;
}

Parameter parameter = model.getParameter(id);
if (parameter != null) {
return parameter;
}

Reaction reaction = model.getReaction(id);
if (reaction != null) {
return reaction;
}

return null;
}

/**
* Returns the variables in a MathML object without local parameter
*
Expand All @@ -824,46 +864,59 @@ public Map<SBase, SBase> getMatching() {
private void getVariables(ListOf<LocalParameter> param, ASTNode node,
List<SBase> variables, int level) {

if (node == null)
{
if (node == null) {
return;
}

// found node with species
if ((node.getChildCount() == 0) && (node.isString()) &&
(node.getType() != Type.NAME_TIME) &&
(node.getType() != Type.NAME_AVOGADRO)) { // TODO - deal with csymbol rateOf as well ?
// found node with species / compartment / parameter / reaction id
if ((node.getChildCount() == 0) && node.isString()
&& (node.getType() != Type.NAME_TIME)
&& (node.getType() != Type.NAME_AVOGADRO)) {

if (!node.isConstant()) {
if (param == null) {
SBase variable=node.getVariable();
if (level==1) {
int insertingPosition = 0;
for (SBase element:variables) {
if (!(element instanceof Parameter) || (!((Parameter)element).isSetValue())) {
insertingPosition++;
}
}
variables.add(insertingPosition, variable);
}
else {
variables.add(variable);

// Try to get the referenced SBase. For identifiers used in
// csymbol rateOf (L3V2), getVariable() may not yet be set,
// so we fall back to resolving the id in the model.
SBase variable = node.getVariable();
if (variable == null) {
String id = node.getName();
if (id != null) {
variable = resolveSBaseFromId(id);
}
} else {
if (!param.contains(node.getVariable())) {
SBase variable=node.getVariable();
if (level==1) {
int insertingPosition=0;
for (SBase element:variables) {
if (!(element instanceof Parameter) ||
(!((Parameter) element).isSetValue())) {
}

if (variable != null) {
if (param == null) {
// Global identifier
if (level == 1) {
int insertingPosition = 0;
for (SBase element : variables) {
if (!(element instanceof Parameter)
|| (!((Parameter) element).isSetValue())) {
insertingPosition++;
}
}
variables.add(insertingPosition, variable);
}
else {
} else {
variables.add(variable);
}
} else {
// Exclude local parameters
if (!param.contains(variable)) {
if (level == 1) {
int insertingPosition = 0;
for (SBase element : variables) {
if (!(element instanceof Parameter)
|| (!((Parameter) element).isSetValue())) {
insertingPosition++;
}
}
variables.add(insertingPosition, variable);
} else {
variables.add(variable);
}
}
}
}
}
Expand Down Expand Up @@ -933,4 +986,4 @@ private void updateMatching(List<Node<SBase>> path) {
}
}

}
}
62 changes: 62 additions & 0 deletions core/test/org/sbml/jsbml/validator/OverdeterminationValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package org.sbml.jsbml.validator;

import static org.junit.Assert.assertTrue;

import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;

import org.junit.Test;
import org.sbml.jsbml.ASTNode;
import org.sbml.jsbml.Model;
import org.sbml.jsbml.Reaction;
import org.sbml.jsbml.SBase;

/**
* Tests for {@link OverdeterminationValidator}.
*/
public class OverdeterminationValidatorTest {

/**
* In SBML Level 3 Version 2, the csymbol {@code rateOf} can target
* a reaction identifier. The {@link OverdeterminationValidator}
* must treat that identifier as a referenced variable in the MathML
* expression.
*
* This test checks that when an {@link ASTNode} refers to a reaction
* by its id, and {@code getVariable()} is not set, the validator still
* resolves the identifier via the model and collects the reaction as
* a variable.
*/
@Test
public void testGetVariablesResolvesReactionIdWhenVariableNotSet() throws Exception {
// Create minimal L3V2 model with one reaction
Model model = new Model(3, 2);
model.setId("m");
Reaction r1 = model.createReaction();
r1.setId("R1");

// Create an AST node that refers to the reaction by its id.
// In some L3V2 rateOf constructs, getVariable() may not be set
// on such nodes; the validator must then resolve the id via the model.
ASTNode nameNode = new ASTNode("R1");

OverdeterminationValidator validator = new OverdeterminationValidator(model);

// Prepare list to receive variables
List<SBase> vars = new ArrayList<SBase>();

// Call the private getVariables(..) method via reflection
Method m = OverdeterminationValidator.class.getDeclaredMethod(
"getVariables",
org.sbml.jsbml.ListOf.class,
ASTNode.class,
List.class,
int.class);
m.setAccessible(true);
m.invoke(validator, null, nameNode, vars, model.getLevel());

assertTrue("The reaction referenced by id must be collected as a variable",
vars.contains(r1));
}
}
Loading