Skip to content

Partial upgrade of libsedml, fixed cli plot bugs#1627

Merged
CodeByDrescher merged 32 commits intomasterfrom
bsiosim_visual_results_fixes
May 1, 2026
Merged

Partial upgrade of libsedml, fixed cli plot bugs#1627
CodeByDrescher merged 32 commits intomasterfrom
bsiosim_visual_results_fixes

Conversation

@CodeByDrescher
Copy link
Copy Markdown
Contributor

@CodeByDrescher CodeByDrescher commented Jan 30, 2026

Summary

Two largely independent changes bundled together:

  1. Bug fixes for SED-ML/CLI visual results (the user-visible motivation)
  2. Partial upgrade of the embedded jlibsedml fork to support those fixes —
    package reorg into org.jlibsedml.components.*, SEDML→SedML naming,
    new first-class Axis and Plot abstractions, generic ListOf<T>,
    and a SedMLDataContainer facade with model-pruning support.

Bug fixes

Plot rendering (vcell-cli)

  • Legend no longer eats the plot when a series has many curves — legend
    font now scales down (1pt per 100 chars of label, floor 8pt).
    Results2DLinePlot.java
  • Series labels are now picked correctly — three-tier fallback
    (curve name → curve id → data-generator name/id) and explicit "Time"
    detection for the x-axis.
    PlottingDataExtractor.java
  • Spatial results task-id collisions fixed — keys are now SId, not raw
    String.
    SpatialResultsConverter.java, ReorganizedSpatialResults.java

SED-ML import/export

  • Variable constructor arg-order bug — XPath and task reference were
    swapped, producing wrong variable mappings in exported SED-ML.
    SEDMLExporter.java
  • NPE when SBML symbol mapping is null — explicit loop replaces
    stream.toMap, which fails on null values.
    SedMLImporter.java
  • Multi-application BioModels now import correctlySBMLImporter
    is keyed by SimulationContext, not BioModel, so each application
    in a BioModel gets its own symbol table.
    SedMLImporter.java
  • Preserve insertion order of imported models (LinkedHashMap).
  • Clearer/findable errors during SED-ML import (explicit exceptions
    instead of silent failures). BiosimulationLog.java

Defensive fixes

  • cbit.vcell.math.Variable — null-checks and identity-validation
    unified into rename(); constructor delegates so validation can't be
    bypassed.
  • SEDMLChooserPanel — tolerates SED-ML where a task references a
    missing model/simulation rather than NPE-ing the dialog.
  • Regex character-range typo ([a-zA-z…][a-zA-Z…]) flagged by
    GitHub Advanced Security in SedMLReader.

jlibsedml partial upgrade

The local jlibsedml fork was reorganised to support the fixes above
and to model SED-ML constructs that were previously missing.

Restructure (mechanical):

  • Classes moved into org.jlibsedml.components.{model,output,task, simulation,algorithm,dataGenerator,listOfConstructs}.
  • SEDML*SedML* naming throughout (SedMLDocument, SedMLReader,
    SedMLWriter, SedMLImporter, SedMLUtil, SedMLTags, …).

New infrastructure:

  • First-class Axis hierarchy (Axis, XAxis, YAxis, ZAxis,
    RightYAxis) with LINEAR/LOG10 type — needed for proper axis-label
    picking.
  • Abstract Plot superclass with axis, legend, and dimension
    state; cloneable.
  • Generic ListOf<T> with order-preserving storage, id-based
    lookup, recursive searchFor, and duplicate-id guard.
  • SedMLDataContainer — facade wrapping a SedML plus namespace
    and file-path metadata. Adds pruneSedML() to remove dangling
    task/datagenerator/output references (with cascading cleanup of
    nested repeated-tasks) and findById convenience accessors.
  • New Calculation interface; new Analysis simulation type.
  • equals/hashCode on SedBase; clone() deep-copy on SedML
    and all components.

Test infrastructure

  • SSIMComparisonTool — perceptual image comparison (Structural
    Similarity Index) replacing pixel-exact baseline matching for plot
  • New Calculation interface; new Analysis simulation type.
  • equals/hashCode on SedBase; clone() deep-copy on SedML
    and all components.

Test infrastructure

  • SSIMComparisonTool — perceptual image comparison (Structural
    Similarity Index) replacing pixel-exact baseline matching for plot
    tests. A "Modified SSIM" variant applies a 5×5 Gaussian blur before
    the structure term to tolerate sub-pixel font/rendering differences.
    Threshold set to 0.90 MSSIM after iteration.
  • New baseline PNGs: plot_result_0.png, plot_result_1.png,
    parabolic.png (replacing plot_0/1.png, Parabolic.png).
  • TestComponents.java — first jlibsedml unit test, exercises
    Axis parsing through SedMLReader.

Test plan

  • mvn test -Dgroups="Fast" passes
  • SED-ML import/export integration tests
    (SEDML_VCML_IT, SEDML_SBML_IT, BSTS_IT)
  • Spot-check legend rendering on a high-species-count BioModel
  • Spot-check spatial results export end-to-end
  • Re-comment the tmate debug step in .github/workflows/ci_cd.yml
    before merge

Risk

  • The jlibsedml package reorg touches ~200 files but is largely
    mechanical; cross-module call sites have been adapted.
  • The BioModel→SBMLImporter to SimulationContext→SBMLImporter
    change is a real semantic shift — worth a careful look at
    SedMLImporter callers in test runs.

Comment thread vcell-core/src/main/java/org/jlibsedml/SedMLReader.java Fixed
@CodeByDrescher CodeByDrescher force-pushed the bsiosim_visual_results_fixes branch 3 times, most recently from 67af9ee to 006b089 Compare February 3, 2026 21:43
@CodeByDrescher CodeByDrescher force-pushed the bsiosim_visual_results_fixes branch 2 times, most recently from 84f97da to 5823a4c Compare April 15, 2026 15:26
@CodeByDrescher CodeByDrescher force-pushed the bsiosim_visual_results_fixes branch from 2fed76e to 1a99491 Compare April 15, 2026 18:07
Copy link
Copy Markdown
Member

@jcschaff jcschaff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fail early when SimulationContext doesn't have a SBMLSymbolMapping in the SedMLImporter. make SedMLImporter.getSBMLSymbolMapping(simContext) throw IllegalStateException("no SBML importer registered for sim context " + simContext)

Comment thread vcell-cli/src/main/java/org/vcell/cli/run/plotting/PlottingDataExtractor.java Outdated
@CodeByDrescher CodeByDrescher force-pushed the bsiosim_visual_results_fixes branch from d740e7c to bc21233 Compare April 30, 2026 18:34
@CodeByDrescher
Copy link
Copy Markdown
Contributor Author

Note that issue #1673 was created to point out the flaws this PR leaves CLI mode in. Code is functional and passes all tests, but needs return work when priorities come around again.

Copy link
Copy Markdown
Member

@jcschaff jcschaff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, refactor of VCML/SBML during sedml importing needed but beyond scope of this PR, see issue #1673

@CodeByDrescher CodeByDrescher merged commit 1b68083 into master May 1, 2026
13 checks passed
@CodeByDrescher CodeByDrescher deleted the bsiosim_visual_results_fixes branch May 1, 2026 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants