Skip to content

Commit

Permalink
add observer for getting timepoints and processed results
Browse files Browse the repository at this point in the history
  • Loading branch information
hemilpanchiwala committed Jun 26, 2020
1 parent 4ce674a commit 8b112e9
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 14 deletions.
35 changes: 25 additions & 10 deletions src/main/java/org/simulator/math/odes/AbstractDESSolver.java
Expand Up @@ -38,6 +38,7 @@
import org.apache.commons.math.ode.events.EventHandler;
import org.simulator.math.Mathematics;
import org.simulator.math.odes.MultiTable.Block.Column;
import org.simulator.sbml.SBMLinterpreter;

/**
* This Class represents an abstract solver for event-driven DES
Expand Down Expand Up @@ -123,6 +124,12 @@ public static long getSerialversionuid() {
*/
public static final String PROGRESS = "progress";

/**
* Key used when informing listeners about change in the result by this
* solver.
*/
public static final String RESULT = "result";

/**
* Initialize with default integration step size and non-negative attribute
* {@code true}.
Expand Down Expand Up @@ -272,8 +279,8 @@ boolean checkSolution(double[] currentChange, double[] yPrev) {
return !unstableFlag;
}

/* (non-Javadoc)
* @see java.lang.Object#clone()
/**
* {@inheritDoc}
*/
@Override
public abstract AbstractDESSolver clone();
Expand Down Expand Up @@ -431,16 +438,18 @@ public int eventOccurred(double t, double[] y, boolean increasing)
return STOP;
}

/* (non-Javadoc)
* @see org.sbml.simulator.math.odes.DESSolver#firePropertyChanged(double, double)
/**
* {@inheritDoc}
*/
@Override
public void firePropertyChange(double oldValue, double newValue /*, double[] currResult, double[] additionalResult*/) {
public void firePropertyChange(double oldValue, double newValue, double[] currResult) {

This comment has been minimized.

Copy link
@draeger

draeger Jun 26, 2020

Collaborator

It is not clear to me why we have oldValue and newValue. This method is, to my knowledge, used when some value is replaced with a new version. But here, we are dealing with a new result vector in each iteration. It seems to me we should implement a Listener class so that it only has a method that receives some kind of an event object, which can encapsulate the information we need, i.e., the latest time point and the latest result vector.

This comment has been minimized.

Copy link
@hemilpanchiwala

hemilpanchiwala Jun 27, 2020

Author Owner

@draeger, here, I think I need to only remove the oldValue from the parameters of this method. Then, after this, I am making two PropertyChangeEvents with different propertyNames namely PROGRESS (the updated timepoint) and RESULT (the updated result) in the firePropertyChange method. And wherever I want to access the values onChange of the property (like in SBMLinterpreter), I implemented the PropertyChangeListener to that class, and in the overrided propertyChange method, I set the changed values of the variables.

if (!listenerList.isEmpty()) {
PropertyChangeEvent evt = new PropertyChangeEvent(this, PROGRESS, oldValue, newValue);
PropertyChangeEvent evt1 = new PropertyChangeEvent(this, PROGRESS, oldValue, newValue);
PropertyChangeEvent evt2 = new PropertyChangeEvent(this, RESULT, currResult, currResult);
// logger.info(String.format("Progress: %s %%", StringTools.toString(newValue)));
for (PropertyChangeListener listener : listenerList) {
listener.propertyChange(evt);
listener.propertyChange(evt1);
listener.propertyChange(evt2);
}
}
}
Expand Down Expand Up @@ -770,10 +779,12 @@ public MultiTable solve(DESystem DES, double[] initialValues, double timeBegin,
if (fastFlag) {
result[0] = computeSteadyState(((FastProcessDESystem) DES), result[0], timeBegin);
}
addPropertyChangeListener((SBMLinterpreter) DES);

// execute events that trigger at 0.0
processEvents((EventDESystem) DES, 0d, 0d, result[0]);
System.arraycopy(result[0], 0, yTemp, 0, yTemp.length);
firePropertyChange(0d, 0d, result[0]);
for (int i = 1; (i < result.length) && (!Thread.currentThread().isInterrupted()); i++) {
double oldT = t;
System.arraycopy(yTemp, 0, yPrev, 0, yTemp.length);
Expand All @@ -790,7 +801,7 @@ public MultiTable solve(DESystem DES, double[] initialValues, double timeBegin,
// if (logger.getLevel().intValue() < Level.INFO.intValue()) {
// logger.fine("additional results: " + Arrays.toString(v));
// }
firePropertyChange(oldT * intervalFactor, t * intervalFactor);
firePropertyChange(oldT * intervalFactor, t * intervalFactor, yTemp);
}
return data;
}
Expand Down Expand Up @@ -837,9 +848,11 @@ public MultiTable solve(DESystem DES, double[] initialValues, double[] timePoint
result[0] = computeSteadyState(((FastProcessDESystem) DES), result[0], timePoints[0]);
}

addPropertyChangeListener((SBMLinterpreter) DES);
// execute events that trigger at 0.0
processEvents((EventDESystem) DES, 0d, 0d, result[0]);
System.arraycopy(result[0], 0, yTemp, 0, result[0].length);
firePropertyChange(0d, 0d, result[0]);
for (int i = 1; (i < timePoints.length) && (!Thread.currentThread().isInterrupted()); i++) {
h = stepSize;
// h = h / 10;
Expand All @@ -866,7 +879,7 @@ public MultiTable solve(DESystem DES, double[] initialValues, double[] timePoint
// if (logger.getLevel().intValue() < Level.INFO.intValue()) {
// logger.fine("additional results: " + Arrays.toString(v));
// }
firePropertyChange(timePoints[i - 1] * intervalFactor, timePoints[i] * intervalFactor);
firePropertyChange(timePoints[i - 1], timePoints[i], yTemp);
t = timePoints[i];
}
return data;
Expand All @@ -881,6 +894,7 @@ public MultiTable solve(DESystem DES, MultiTable.Block initConditions, double[]
if (DES instanceof DelayedDESystem) {
((DelayedDESystem) DES).registerDelayValueHolder(this);
}
addPropertyChangeListener((SBMLinterpreter) DES);
double[] timePoints = initConditions.getTimePoints();

// of items to be simulated, this will cause a problem!
Expand Down Expand Up @@ -909,6 +923,7 @@ public MultiTable solve(DESystem DES, MultiTable.Block initConditions, double[]
double[] change = new double[DES.getDimension()];
double t = timePoints[0];
double v[] = additionalResults(DES, t, result[0], data, 0);
firePropertyChange(0d, 0d, result[0]);
for (i = 1; (i < timePoints.length) && (!Thread.currentThread().isInterrupted()); i++) {
double h = stepSize;
if (!missingIds.isEmpty()) {
Expand Down Expand Up @@ -940,7 +955,7 @@ public MultiTable solve(DESystem DES, MultiTable.Block initConditions, double[]
// if ((logger != null) && (logger.getLevel().intValue() < Level.INFO.intValue())) {
// logger.fine("additional results: " + Arrays.toString(v));
// }
firePropertyChange(timePoints[i - 1] * intervalFactor, timePoints[i] * intervalFactor);
firePropertyChange(timePoints[i - 1] * intervalFactor, timePoints[i] * intervalFactor, yTemp);
t = timePoints[i];
}
return data;
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/simulator/math/odes/DESSolver.java
Expand Up @@ -54,12 +54,14 @@ public interface DESSolver extends Cloneable, Serializable {

/**
* Tell each listener that property value changed. OldValue and newValue are
* the old and current time point of simulation, respectively.
* the old and current time point of simulation, respectively. CurrResult
* is the row of the result at the current time point of simulation.
*
* @param oldValue
* @param newValue
* @param currResult
*/
public void firePropertyChange(double oldValue, double newValue);
public void firePropertyChange(double oldValue, double newValue, double[] currResult);

/**
* For details about the Kinetic Simulation Algorithm Ontology (KiSAO) see
Expand Down
45 changes: 43 additions & 2 deletions src/main/java/org/simulator/sbml/SBMLinterpreter.java
Expand Up @@ -24,6 +24,8 @@
*/
package org.simulator.sbml;

import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -58,7 +60,6 @@
import org.sbml.jsbml.Species;
import org.sbml.jsbml.SpeciesReference;
import org.sbml.jsbml.Symbol;
import org.sbml.jsbml.util.ValuePair;
import org.sbml.jsbml.validator.ModelOverdeterminedException;
import org.sbml.jsbml.validator.OverdeterminationValidator;
import org.simulator.math.RNG;
Expand Down Expand Up @@ -108,7 +109,7 @@
*/
public class SBMLinterpreter
implements DelayedDESystem, EventDESystem, FastProcessDESystem,
RichDESystem, SBMLValueHolder {
RichDESystem, SBMLValueHolder, PropertyChangeListener {

/**
* A {@link Logger}.
Expand Down Expand Up @@ -480,6 +481,17 @@ public int getConstraintListenerCount() {

private boolean containsDelays;

/**
* The value of the last time point processed
*/
private double lastTimePoint;

/**
* An array of the concentration of each species at last processed time point
* within the model system.
*/
private double[] lastTimePointResult;


/**
* <p>
Expand Down Expand Up @@ -571,6 +583,8 @@ public SBMLinterpreter(Model model, double defaultSpeciesValue, double defaultPa
reactionReversible = new boolean[model.getReactionCount()];
initialValues = new double[Y.length];
nodes = new LinkedList<ASTNode>();
lastTimePoint = 0d;
lastTimePointResult = new double[Y.length];
init(true, defaultSpeciesValue, defaultParameterValue, defaultCompartmentValue, amountHash);
}

Expand Down Expand Up @@ -2692,4 +2706,31 @@ public Map<String, Boolean> getConstantHash() {
public double[] getY() {
return Y;
}

@Override
public void propertyChange(PropertyChangeEvent propertyChangeEvent) {

if (propertyChangeEvent.getPropertyName().equals("result")){

This comment has been minimized.

Copy link
@draeger

draeger Aug 5, 2020

Collaborator

Property names should be addressed using String literals in form of public static final String variables to avoid typos.

This comment has been minimized.

Copy link
@hemilpanchiwala

hemilpanchiwala Aug 6, 2020

Author Owner

@draeger, I am using this string only once in the entire class. So, should we define it as a variable just for one time usage?

This comment has been minimized.

Copy link
@draeger

draeger Aug 6, 2020

Collaborator

It is always safer and makes it easier to reuse it later on when needed. We never know if any third-party user of the library wants to use the same property name for a customized listener or so.

setLastTimePointResult((double[]) propertyChangeEvent.getNewValue());
}else {
setLastTimePoint((Double) propertyChangeEvent.getNewValue());
}

}

public double getLastTimePoint() {

This comment has been minimized.

Copy link
@draeger

draeger Jun 26, 2020

Collaborator

To me, "last" sounds like the last one in the entire table. But I think, you are here requesting the latest?

return lastTimePoint;
}

public void setLastTimePoint(double lastTimePoint) {
this.lastTimePoint = lastTimePoint;
}

public double[] getLastTimePointResult() {

This comment has been minimized.

Copy link
@draeger

draeger Jun 26, 2020

Collaborator

Same here, it is probably just the latest result?

return lastTimePointResult;
}

public void setLastTimePointResult(double[] lastTimePointResult) {

This comment has been minimized.

Copy link
@draeger

draeger Jun 26, 2020

Collaborator

Why is there a public method for setting the result? This could be misused from external callers. Same also for the time point.

This comment has been minimized.

Copy link
@hemilpanchiwala

hemilpanchiwala Jun 26, 2020

Author Owner

Will update to private.

this.lastTimePointResult = lastTimePointResult;
}
}

0 comments on commit 8b112e9

Please sign in to comment.