Jim review modifications

Fixes include logging, raising plot-pixel accurate thresholds, making code more convenient and/or easier to read / conceptualize (plus some improvements!).

Author: CodeByDrescher

vcell-cli/src/main/java/org/vcell/cli/run/ExecutionJob.java

@@ -141,7 +141,7 @@ public void executeArchive(boolean isBioSimSedml) throws BiosimulationsHdfWriter
 
     private void executeSedmlDocument(String sedmlLocation, HDF5ExecutionResults cumulativeHdf5Results) throws IOException, PreProcessingException, ExecutionException {
         BiosimulationLog.instance().updateSedmlDocStatusYml(sedmlLocation, BiosimulationLog.Status.QUEUED);
-        SedmlJob job = new SedmlJob(sedmlLocation, this.omexHandler, this.inputFile, this.outputDir, this.sedmlPath2d3d.toString(), this.cliRecorder, this.bKeepTempFiles, this.bExactMatchOnly, this.bSmallMeshOverride);
+        SedMLJob job = new SedMLJob(sedmlLocation, this.omexHandler, this.inputFile, this.outputDir, this.sedmlPath2d3d.toString(), this.cliRecorder, this.bKeepTempFiles, this.bExactMatchOnly, this.bSmallMeshOverride);
         this.logOmexMessage.append("Processing ").append(job.SEDML_NAME).append(". ");
         SedmlStatistics stats = job.preProcessDoc();
         boolean hasSucceeded = job.simulateSedml(cumulativeHdf5Results);

vcell-cli/src/main/java/org/vcell/cli/run/RunUtils.java

@@ -179,8 +179,8 @@ public static HashMap<SId, File> generateReportsAsCSV(SedMLDataContainer sedmlCo
                 List<DataSet> datasets = sedmlReport.getDataSets();
                 Map<DataSet, DataGenerator> dataGeneratorMapping = new LinkedHashMap<>();
                 for (DataSet dataset : datasets) {
-                    SedBase maybeDataGenerator = sedML.searchInDataGeneratorsFor(dataset.getDataReference());
-                    if (!(maybeDataGenerator instanceof DataGenerator referencedGenerator)) throw new RuntimeException("SedML DataGenerator referenced by report is missing!");
+                    DataGenerator referencedGenerator = sedmlContainer.findDataGeneratorById(dataset.getDataReference());
+                    if (null == referencedGenerator) throw new IllegalArgumentException("Unable to find data generator referenced in dataset: " + dataset.getDataReference());
                     if (!organizedNonSpatialResults.containsKey(referencedGenerator)) break;
                     dataGeneratorMapping.put(dataset, referencedGenerator);
                 }

…ain/java/org/vcell/cli/run/SedmlJob.java …ain/java/org/vcell/cli/run/SedMLJob.javavcell-cli/src/main/java/org/vcell/cli/run/SedmlJob.java renamed to vcell-cli/src/main/java/org/vcell/cli/run/SedMLJob.java vcell-cli/src/main/java/org/vcell/cli/run/SedmlJob.java renamed to vcell-cli/src/main/java/org/vcell/cli/run/SedMLJob.java

@@ -1,6 +1,5 @@
 package org.vcell.cli.run;
 
-import cbit.vcell.biomodel.BioModel;
 import cbit.vcell.resource.OperatingSystemInfo;
 import cbit.vcell.xml.ExternalDocInfo;
 import org.apache.commons.io.FilenameUtils;
@@ -32,7 +31,6 @@
 import org.vcell.cli.run.plotting.PlottingDataExtractor;
 import org.vcell.cli.run.plotting.ChartCouldNotBeProducedException;
 import org.vcell.cli.run.plotting.Results2DLinePlot;
-import org.vcell.sbml.vcell.SBMLSymbolMapping;
 import org.vcell.sbml.vcell.lazy.LazySBMLNonSpatialDataAccessor;
 import org.vcell.sbml.vcell.lazy.LazySBMLSpatialDataAccessor;
 import org.vcell.sedml.log.BiosimulationLog;
@@ -53,8 +51,8 @@
 /**
  * Class that deals with the processing quest of a sedml file.
  */
-public class SedmlJob {
-    private final static Logger logger = LogManager.getLogger(SedmlJob.class);
+public class SedMLJob {
+    private final static Logger logger = LogManager.getLogger(SedMLJob.class);
 
     private final boolean SHOULD_KEEP_TEMP_FILES,
             ACCEPT_EXACT_MATCH_ONLY, SHOULD_OVERRIDE_FOR_SMALL_MESH;
@@ -83,7 +81,7 @@ public class SedmlJob {
      * @param bExactMatchOnly enforces a KISAO match, with no substitution
      * @param bSmallMeshOverride whether to use small meshes or standard meshes.
      */
-    public SedmlJob(String sedmlLocation, OmexHandler omexHandler, File masterOmexArchive,
+    public SedMLJob(String sedmlLocation, OmexHandler omexHandler, File masterOmexArchive,
                     String resultsDirPath, String pathToPlotsDirectory, CLIRecordable cliRecorder,
                     boolean bKeepTempFiles, boolean bExactMatchOnly, boolean bSmallMeshOverride){
         final String SAFE_WINDOWS_FILE_SEPARATOR = "\\\\";
@@ -127,13 +125,13 @@ public SedmlStatistics preProcessDoc() throws IOException, PreProcessingExceptio
             logger.info("Initializing and Pre-Processing SedML document: {}", this.SEDML_NAME);
             biosimLog.updateSedmlDocStatusYml(this.SEDML_LOCATION, BiosimulationLog.Status.RUNNING);
             try {
-                this.sedml = SedmlJob.getSedMLFile(this.SEDML_NAME_SPLIT, this.MASTER_OMEX_ARCHIVE);
+                this.sedml = SedMLJob.getSedMLFile(this.SEDML_NAME_SPLIT, this.MASTER_OMEX_ARCHIVE);
             } catch (Exception e) {
                 String prefix = "SedML pre-processing for " + this.SEDML_LOCATION + " failed";
                 this.logDocumentError = prefix + ": " + e.getMessage();
                 Tracer.failure(e, prefix);
                 this.reportProblem(e);
-                this.somethingFailed = SedmlJob.somethingDidFail();
+                this.somethingFailed = SedMLJob.somethingDidFail();
                 biosimLog.updateSedmlDocStatusYml(this.SEDML_LOCATION, BiosimulationLog.Status.FAILED);
                 span.close();
                 throw new PreProcessingException(prefix, e);
@@ -191,8 +189,6 @@ public SedmlStatistics preProcessDoc() throws IOException, PreProcessingExceptio
             }
 
 
-        } catch(Exception e){
-            throw e;
         } finally {
             if (span != null) span.close();
         }
@@ -231,10 +227,10 @@ private void runSimulations(SolverHandler solverHandler) throws ExecutionExcepti
         RunUtils.drawBreakLine("-", 100);
         try {
             span = Tracer.startSpan(Span.ContextType.SIMULATIONS_RUN, "runSimulations", null);
-            Pair<SedMLDataContainer, Map<BioModel, SBMLSymbolMapping>> initializedModelPair = solverHandler.initialize(externalDocInfo, this.sedml, this.ACCEPT_EXACT_MATCH_ONLY);
-            if (!this.sedml.equals(initializedModelPair.one)){
+            SolverHandler.Configuration initializedModelPair = solverHandler.initialize(externalDocInfo, this.sedml, this.ACCEPT_EXACT_MATCH_ONLY);
+            if (!this.sedml.equals(initializedModelPair.postInitializedSedml())){
                 logger.warn("Importer returned modified SedML to process; now using modified SedML");
-                this.sedml = initializedModelPair.one;
+                this.sedml = initializedModelPair.postInitializedSedml();
             }
             Map<AbstractTask, BiosimulationLog.Status> taskResults =  solverHandler.simulateAllTasks(this.CLI_RECORDER,
                     this.OUTPUT_DIRECTORY_FOR_CURRENT_SEDML, this.SEDML_LOCATION, this.SHOULD_KEEP_TEMP_FILES, this.SHOULD_OVERRIDE_FOR_SMALL_MESH);
@@ -398,7 +394,7 @@ private void indexHDF5Data(Map<DataGenerator, ValueHolder<LazySBMLNonSpatialData
 
     // This method is a bit weird; it uses a temp file as a reference to compare against while getting the file straight from the archive.
     private static SedMLDataContainer getSedMLFile(String[] tokenizedPathToSedml, File inputFile) throws XMLException, IOException {
-        Path convertedPath = SedmlJob.getRelativePath(tokenizedPathToSedml);
+        Path convertedPath = SedMLJob.getRelativePath(tokenizedPathToSedml);
         if (convertedPath == null) throw new RuntimeException("Was not able to get relative path to " + inputFile.getName());
         String identifyingPath = FilenameUtils.separatorsToUnix(convertedPath.toString());
         try (FileInputStream omexStream = new FileInputStream(inputFile)) {

vcell-cli/src/main/java/org/vcell/cli/run/SolverHandler.java

@@ -23,7 +23,6 @@
 import cbit.vcell.xml.ExternalDocInfo;
 
 import org.jlibsedml.components.SId;
-import org.jlibsedml.components.SedBase;
 import org.jlibsedml.components.SedML;
 import org.jlibsedml.components.task.AbstractTask;
 import org.jlibsedml.components.dataGenerator.DataGenerator;
@@ -39,7 +38,6 @@
 import org.jlibsedml.components.simulation.UniformTimeCourse;
 import org.jlibsedml.components.Variable;
 import org.jlibsedml.XPathTarget;
-import org.jlibsedml.XMLException;
 import org.jlibsedml.modelsupport.SBMLSupport;
 
 import org.jmathml.ASTNode;
@@ -54,7 +52,6 @@
 import org.apache.commons.lang.NotImplementedException;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.vcell.util.Pair;
 
 import java.beans.PropertyVetoException;
 import java.io.*;
@@ -102,18 +99,22 @@ private static void sanityCheck(BioModel bioModel) throws SEDMLImportException {
         }
     }
 
-    public Pair<SedMLDataContainer, Map<BioModel, SBMLSymbolMapping>> initialize(ExternalDocInfo externalDocInfo, SedMLDataContainer providedSedmlContainer, boolean disallowModifiedImport)
+    public Configuration initialize(ExternalDocInfo externalDocInfo, SedMLDataContainer initialSedmlContainer, boolean exactMatchOnly)
             throws ExpressionException, SEDMLImportException {
         cbit.util.xml.VCLogger sedmlImportLogger = new LocalLogger();
 
         //String outDirRoot = outputDirForSedml.toString().substring(0, outputDirForSedml.toString().lastIndexOf(System.getProperty("file.separator")));
         SedMLDataContainer actionableSedmlContainer;
         Map<BioModel, SBMLSymbolMapping> bioModelMapping;
-        SedMLImporter sedmlImporter = new SedMLImporter(sedmlImportLogger, disallowModifiedImport);
+        SedMLImporter sedmlImporter = new SedMLImporter(sedmlImportLogger, new SedMLImporter.StrictnessPolicy(
+                true,
+                SedMLImporter.StrictnessPolicy.MultipleSubTaskPolicy.CONDENSE_ELSE_REMOVE,
+                exactMatchOnly ? SedMLImporter.StrictnessPolicy.SolverMatchPolicy.STRICT_MATCH_OR_REJECT : SedMLImporter.StrictnessPolicy.SolverMatchPolicy.SUNDIALS_AS_LAST_RESORT
+        ));
         this.modelReportingName  = org.vcell.util.FileUtils.getBaseName(externalDocInfo.getFile().getAbsolutePath());
 
         try {
-            actionableSedmlContainer = sedmlImporter.initialize(externalDocInfo.getFile(), providedSedmlContainer);
+            actionableSedmlContainer = sedmlImporter.initialize(externalDocInfo.getFile(), initialSedmlContainer);
         } catch (Exception e) {
             String errMessage = "Unable to prepare SED-ML for conversion into BioModel(s)";
             String formattedError = String.format("%s, failed with error: %s", errMessage, e.getMessage());
@@ -131,7 +132,7 @@ public Pair<SedMLDataContainer, Map<BioModel, SBMLSymbolMapping>> initialize(Ext
 
         this.countBioModels = bioModelMapping.size();
 
-        SedML sedML = providedSedmlContainer.getSedML();
+        SedML sedML = initialSedmlContainer.getSedML();
         Set <AbstractTask> topmostTasks = new LinkedHashSet<> ();
         for(BioModel bioModel : bioModelMapping.keySet()) {
 			Simulation[] sims = bioModel.getSimulations();
@@ -141,8 +142,8 @@ public Pair<SedMLDataContainer, Map<BioModel, SBMLSymbolMapping>> initialize(Ext
 				}
 				TempSimulation tempSimulation = new TempSimulation(sim,false);
 				String importedTaskId = tempSimulation.getImportedTaskID();
-                SedBase foundElement = sedML.searchInTasksFor(new SId(importedTaskId));
-                if (!(foundElement instanceof AbstractTask abstractTask)) throw new RuntimeException("Imported task id " + importedTaskId + " is not an AbstractTask.");
+                AbstractTask abstractTask = initialSedmlContainer.findAbstractTaskById(new SId(importedTaskId));
+                if (null == abstractTask) throw new RuntimeException("Imported task id " + importedTaskId + " is not an AbstractTask.");
 				this.tempSimulationToTaskMap.put(tempSimulation, abstractTask);
 				this.taskToTempSimulationMap.put(abstractTask,  tempSimulation);
 				this.origSimulationToTempSimulationMap.put(sim, tempSimulation);
@@ -156,8 +157,8 @@ public Pair<SedMLDataContainer, Map<BioModel, SBMLSymbolMapping>> initialize(Ext
             for(AbstractTask at : sedML.getTasks()) {
                 if(!(at instanceof RepeatedTask rt)) continue;
                 for (SubTask entry : rt.getSubTasks()) {
-                    SedBase foundElement = sedML.searchInTasksFor(entry.getTask());
-                    if (!(foundElement instanceof AbstractTask subTaskTarget)) throw new RuntimeException("Subtask (id=" + entry.getId().string() + " ) does not reference an AbstractTask.");
+                    AbstractTask subTaskTarget = initialSedmlContainer.findAbstractTaskById(entry.getTask());
+                    if (null == subTaskTarget) throw new RuntimeException("Subtask (id=" + entry.getId().string() + " ) does not reference an AbstractTask.");
                     subTasks.add(subTaskTarget);
                 }
             }
@@ -177,8 +178,8 @@ public Pair<SedMLDataContainer, Map<BioModel, SBMLSymbolMapping>> initialize(Ext
                 List<AbstractTask> subTasksList = new ArrayList<> ();
                 Task baseTask;
                 if(abstractTask instanceof RepeatedTask repeatedTask) {
-                    subTasksList.addAll(providedSedmlContainer.getActualSubTasks(repeatedTask.getId()));
-                    baseTask = providedSedmlContainer.getBaseTask(repeatedTask.getId());
+                    subTasksList.addAll(initialSedmlContainer.getActualSubTasks(repeatedTask.getId()));
+                    baseTask = initialSedmlContainer.getBaseTask(repeatedTask.getId());
                     if (baseTask == null) throw new RuntimeException("Unable to find base task of repeated task: " + repeatedTask.getId().string() + ".");
                 } else if (abstractTask instanceof Task task) {
                     baseTask = task;
@@ -204,11 +205,11 @@ public Pair<SedMLDataContainer, Map<BioModel, SBMLSymbolMapping>> initialize(Ext
                     // variable id is constructed based on the task id
                     List<DataSet> datasets = rep.getDataSets();
                     for (DataSet dataset : datasets) {
-                        SedBase foundDataGen = sedML.searchInDataGeneratorsFor(dataset.getDataReference());
-                        if (!(foundDataGen instanceof DataGenerator dataGen)) throw new IllegalArgumentException("Unable to find data generator referenced in dataset: " + dataset.getDataReference());
+                        DataGenerator dataGen = initialSedmlContainer.findDataGeneratorById(dataset.getDataReference());
+                        if (null == dataGen) throw new IllegalArgumentException("Unable to find data generator referenced in dataset: " + dataset.getDataReference());
                         for(Variable var : dataGen.getVariables()) {
-                            SedBase foundAbstractTask = sedML.searchInTasksFor(var.getTaskReference());
-                            if (!(foundAbstractTask instanceof AbstractTask task)) throw new IllegalArgumentException("Unable to find task referenced by variable: " + var.getTaskReference());
+                            AbstractTask task = initialSedmlContainer.findAbstractTaskById(var.getTaskReference());
+                            if (null == task) throw new IllegalArgumentException("Unable to find task referenced by variable: " + var.getTaskReference());
                             variableToTaskMap.put(var, task);
                         }
                     }
@@ -269,7 +270,7 @@ public Pair<SedMLDataContainer, Map<BioModel, SBMLSymbolMapping>> initialize(Ext
 			logger.info("Initialization Statistics:\n\t> taskToSimulationMap: {}\n\t> taskToListOfSubTasksMap: {}\n\t> taskToVariableMap: {}\n\t> topTaskToBaseTask: {}\n",
 					this.taskToTempSimulationMap.size(), this.taskToListOfSubTasksMap.size(), this.taskToVariableMap.size(), this.topTaskToBaseTask.size());
 		}
-        return new Pair<>(this.initializedSedMLContainer = actionableSedmlContainer, this.bioModelToSBMLMapping = bioModelMapping);
+        return new Configuration(this.initializedSedMLContainer = actionableSedmlContainer, this.bioModelToSBMLMapping = bioModelMapping);
     }
 
 	private static class TempSimulationJob extends SimulationJob {
@@ -385,10 +386,9 @@ public Map<AbstractTask, BiosimulationLog.Status> simulateAllTasks(CLIRecordable
 								// must interpolate data for uniform time course which is not supported natively by the Java solvers
                                 Task baseTask = this.initializedSedMLContainer.getBaseTask(task.getId());
                                 if (baseTask == null) throw new RuntimeException("Unable to find base task");
-                                SedBase elementFound = this.initializedSedMLContainer.getSedML().searchInSimulationsFor(baseTask.getSimulationReference());
-                                if (!(elementFound instanceof org.jlibsedml.components.simulation.Simulation sedmlSim))
-                                    throw new RuntimeException("Unable to find simulation for base task");
-								if (sedmlSim instanceof UniformTimeCourse utcSedmlSim) {
+                                org.jlibsedml.components.simulation.Simulation sedmlSim = this.initializedSedMLContainer.findSimulationById(baseTask.getSimulationReference());
+                                if (null == sedmlSim) throw new RuntimeException("Unable to find simulation for base task");
+                                if (sedmlSim instanceof UniformTimeCourse utcSedmlSim) {
 									odeSolverResultSet = RunUtils.interpolate(odeSolverResultSet, utcSedmlSim);
 									logTaskMessage += "done. Interpolating... ";
 								}
@@ -737,5 +737,7 @@ public static void main(String[] args) throws Exception {
 
 */
     }
+
+    public record Configuration(SedMLDataContainer postInitializedSedml, Map<BioModel, SBMLSymbolMapping> bioModelsToSymbolMappings){}
 }