Merge pull request #267 from sbmlteam/spatial-update-validation

As Mattias noticed, diffusion and advection can apply to parameters.
sbmlteam · Aug 17, 2022 · 6596dfd · 6596dfd
2 parents cddb542 + 4a6d8c1
commit 6596dfd
Show file tree

Hide file tree

Showing 16 changed files with 830 additions and 23 deletions.
diff --git a/src/sbml/packages/spatial/sbml/AdvectionCoefficient.cpp b/src/sbml/packages/spatial/sbml/AdvectionCoefficient.cpp
@@ -750,7 +750,7 @@ AdvectionCoefficient::readAttributes(const XMLAttributes& attributes,
 
       msg += " is '" + mVariable + "', which does not conform to the syntax.";
       log->logPackageError("spatial",
-        SpatialAdvectionCoefficientVariableMustBeSpecies, pkgVersion, level,
+        SpatialAdvectionCoefficientVariableMustBeSpeciesOrParam, pkgVersion, level,
           version, msg, getLine(), getColumn());
     }
   }

diff --git a/src/sbml/packages/spatial/sbml/DiffusionCoefficient.cpp b/src/sbml/packages/spatial/sbml/DiffusionCoefficient.cpp
@@ -968,7 +968,7 @@ DiffusionCoefficient::readAttributes(const XMLAttributes& attributes,
 
       msg += " is '" + mVariable + "', which does not conform to the syntax.";
       log->logPackageError("spatial",
-        SpatialDiffusionCoefficientVariableMustBeSpecies, pkgVersion, level,
+        SpatialDiffusionCoefficientVariableMustBeSpeciesOrParam, pkgVersion, level,
           version, msg, getLine(), getColumn());
     }
   }

diff --git a/src/sbml/packages/spatial/validator/SpatialSBMLError.h b/src/sbml/packages/spatial/validator/SpatialSBMLError.h
@@ -315,7 +315,7 @@ typedef enum
 , SpatialDiffusionCoefficientAllowedCoreAttributes            = 1223401
 , SpatialDiffusionCoefficientAllowedCoreElements              = 1223402
 , SpatialDiffusionCoefficientAllowedAttributes                = 1223403
-, SpatialDiffusionCoefficientVariableMustBeSpecies            = 1223404
+, SpatialDiffusionCoefficientVariableMustBeSpeciesOrParam     = 1223404
 , SpatialDiffusionCoefficientTypeMustBeDiffusionKindEnum      = 1223405
 , SpatialDiffusionCoefficientCoordinateReference1MustBeCoordinateKindEnum= 1223406
 , SpatialDiffusionCoefficientCoordinateReference2MustBeCoordinateKindEnum= 1223407
@@ -327,13 +327,15 @@ typedef enum
 , SpatialDiffusionCoefficientCoordinateReferenceNoYIn1D       = 1223455
 , SpatialDiffusionCoefficientCoordinateReferenceNoZIn2D       = 1223456
 , SpatialNoDiffusionCoefficientOverlap                        = 1223457
+, SpatialDiffusionCoefficientVariableMustNotBeSelf            = 1223458
 , SpatialAdvectionCoefficientAllowedCoreAttributes            = 1223501
 , SpatialAdvectionCoefficientAllowedCoreElements              = 1223502
 , SpatialAdvectionCoefficientAllowedAttributes                = 1223503
-, SpatialAdvectionCoefficientVariableMustBeSpecies            = 1223504
+, SpatialAdvectionCoefficientVariableMustBeSpeciesOrParam     = 1223504
 , SpatialAdvectionCoefficientCoordinateMustBeCoordinateKindEnum= 1223505
 , SpatialAdvectionCoefficientUnits                            = 1223550
 , SpatialAdvectionCoefficientsMustBeUnique                    = 1223551
+, SpatialAdvectionCoefficientVariableMustNotBeSelf            = 1223552
 , SpatialBoundaryConditionAllowedCoreAttributes               = 1223601
 , SpatialBoundaryConditionAllowedCoreElements                 = 1223602
 , SpatialBoundaryConditionAllowedAttributes                   = 1223603

diff --git a/src/sbml/packages/spatial/validator/SpatialSBMLErrorTable.h b/src/sbml/packages/spatial/validator/SpatialSBMLErrorTable.h
@@ -2931,12 +2931,12 @@ static const packageErrorTableEntry spatialErrorTable[] =
   },
 
   // 1223404
-  { SpatialDiffusionCoefficientVariableMustBeSpecies,
-    "The attribute 'variable' must point to Species object.",
+  { SpatialDiffusionCoefficientVariableMustBeSpeciesOrParam,
+    "The attribute 'variable' must point to Species or Parameter object.",
     LIBSBML_CAT_GENERAL_CONSISTENCY,
     LIBSBML_SEV_ERROR,
     "The value of the attribute 'spatial:variable' of a <diffusionCoefficient> "
-    "object must be the identifier of an existing <species> object defined in "
+    "object must be the identifier of an existing <species> or <parameter> object defined in "
     "the enclosing <model> object.",
     { "L3V1 Spatial V1 Section"
     }
@@ -3056,10 +3056,21 @@ static const packageErrorTableEntry spatialErrorTable[] =
 
   // 1223457
   { SpatialNoDiffusionCoefficientOverlap,
-    "No overlapping diffusion coefficients for the same species.",
+    "No overlapping diffusion coefficients for the same species or parameter.",
     LIBSBML_CAT_GENERAL_CONSISTENCY,
     LIBSBML_SEV_ERROR,
-    "Any <species> may only have a single <diffusionCoefficient> that applies to any given cardinal axis or plane.  A <diffusionCoefficient> of type 'anisotropic' applies to the axis it references, and any plane in the <geometry> that contains that axis.  A <diffusionCoefficient> of type 'tensor' applies to the plane defined by the two axes it references.  A <diffusionCoefficient> of type 'isotropic' applies to all axes and planes in the <geometry>.",
+    "Any <species> or <parameter> may only have a single <diffusionCoefficient> that applies to any given cardinal axis or plane.  A <diffusionCoefficient> of type 'anisotropic' applies to the axis it references, and any plane in the <geometry> that contains that axis.  A <diffusionCoefficient> of type 'tensor' applies to the plane defined by the two axes it references.  A <diffusionCoefficient> of type 'isotropic' applies to all axes and planes in the <geometry>.",
+    { "L3V1 Spatial V1 Section"
+    }
+  },
+
+  // 1223458
+  { SpatialDiffusionCoefficientVariableMustNotBeSelf,
+    "The attribute 'variable' must not point to itself.",
+    LIBSBML_CAT_GENERAL_CONSISTENCY,
+    LIBSBML_SEV_ERROR,
+    "The value of the attribute 'spatial:variable' of a <diffusionCoefficient> "
+    "object must not be the identifier of its parent <parameter>.",
     { "L3V1 Spatial V1 Section"
     }
   },
@@ -3101,12 +3112,12 @@ static const packageErrorTableEntry spatialErrorTable[] =
   },
 
   // 1223504
-  { SpatialAdvectionCoefficientVariableMustBeSpecies,
-    "The attribute 'variable' must point to Species object.",
+  { SpatialAdvectionCoefficientVariableMustBeSpeciesOrParam,
+    "The attribute 'variable' must point to Species or Parameter object.",
     LIBSBML_CAT_GENERAL_CONSISTENCY,
     LIBSBML_SEV_ERROR,
     "The value of the attribute 'spatial:variable' of an <advectionCoefficient> "
-    "object must be the identifier of an existing <species> object defined in "
+    "object must be the identifier of an existing <species> or <parameter> object defined in "
     "the enclosing <model> object.",
     { "L3V1 Spatial V1 Section"
     }
@@ -3141,7 +3152,18 @@ static const packageErrorTableEntry spatialErrorTable[] =
     "The 'coordinate' and 'variable' attributes must be unique.",
     LIBSBML_CAT_GENERAL_CONSISTENCY,
     LIBSBML_SEV_ERROR,
-    "No two <advectionCoefficient> elements in the same <model> may have the same values for the attributes 'species:variable' and 'species:coordinate'.  Only one advection coefficient may be defined per species per axis.",
+    "No two <advectionCoefficient> elements in the same <model> may have the same values for the attributes 'spatial:variable' and 'spatial:coordinate'.  Only one advection coefficient may be defined per species (or parameter) per axis.",
+    { "L3V1 Spatial V1 Section"
+    }
+  },
+
+  // 1223552
+  { SpatialAdvectionCoefficientVariableMustNotBeSelf,
+    "The attribute 'variable' must not point to itself.",
+    LIBSBML_CAT_GENERAL_CONSISTENCY,
+    LIBSBML_SEV_ERROR,
+    "The value of the attribute 'spatial:variable' of an <advectionCoefficient> "
+    "object must not be the identifier of its parent <parameter> object.",
     { "L3V1 Spatial V1 Section"
     }
   },

diff --git a/src/sbml/packages/spatial/validator/constraints/SpatialConsistencyConstraints.cpp b/src/sbml/packages/spatial/validator/constraints/SpatialConsistencyConstraints.cpp
@@ -907,7 +907,7 @@ START_CONSTRAINT(SpatialDiffusionCoefficientCoordinateReferenceNoYIn1D, Diffusio
 }
 END_CONSTRAINT
 
-// 1223455
+// 1223456
 START_CONSTRAINT(SpatialDiffusionCoefficientCoordinateReferenceNoZIn2D, DiffusionCoefficient, dc)
 {
   bool fail = false;
@@ -952,20 +952,20 @@ START_CONSTRAINT(SpatialDiffusionCoefficientCoordinateReferenceNoZIn2D, Diffusio
 END_CONSTRAINT
 
 // 1223504
-START_CONSTRAINT(SpatialAdvectionCoefficientVariableMustBeSpecies, AdvectionCoefficient, ac)
+START_CONSTRAINT(SpatialAdvectionCoefficientVariableMustBeSpeciesOrParam, AdvectionCoefficient, ac)
 {
   bool fail = false;
   pre(ac.isSetVariable());
 
-  if (m.getSpecies(ac.getVariable()) == NULL) {
+  if (m.getSpecies(ac.getVariable()) == NULL && m.getParameter(ac.getVariable()) == NULL) {
     fail = true;
     stringstream ss_msg;
     ss_msg << "An <advectionCoefficient>";
     if (ac.isSetId())
     {
       ss_msg << " with id '" << ac.getId() << "'";
     }
-    ss_msg << " references a variable '" << ac.getVariable() << "', which is not the ID of a <species> in the <model>.";
+    ss_msg << " references a variable '" << ac.getVariable() << "', which is not the ID of a <species> or <parameter> in the <model>.";
     msg = ss_msg.str();
   }
 
@@ -974,6 +974,28 @@ START_CONSTRAINT(SpatialAdvectionCoefficientVariableMustBeSpecies, AdvectionCoef
 END_CONSTRAINT
 
 
+// 1223552
+START_CONSTRAINT(SpatialAdvectionCoefficientVariableMustNotBeSelf, AdvectionCoefficient, ac)
+{
+    bool fail = false;
+    pre(ac.isSetVariable());
+    const SBase* parent = ac.getParentSBMLObject();
+    pre(parent != NULL);
+    pre(parent->getId() == ac.getVariable());
+
+    stringstream ss_msg;
+    ss_msg << "An <advectionCoefficient>";
+    if (ac.isSetId())
+    {
+        ss_msg << " with id '" << ac.getId() << "'";
+    }
+    ss_msg << " references its parent parameter '" << ac.getVariable() << "'.";
+    msg = ss_msg.str();
+    inv(false);
+}
+END_CONSTRAINT
+
+
 // 1223604
 START_CONSTRAINT(SpatialBoundaryConditionVariableMustBeSpecies, BoundaryCondition, bc)
 {
@@ -2611,22 +2633,43 @@ END_CONSTRAINT
 
 
 // 1223404
-START_CONSTRAINT(SpatialDiffusionCoefficientVariableMustBeSpecies, DiffusionCoefficient, dc)
+START_CONSTRAINT(SpatialDiffusionCoefficientVariableMustBeSpeciesOrParam, DiffusionCoefficient, dc)
 {
   pre(dc.isSetVariable());
   string variable = dc.getVariable();
-  pre(m.getSpecies(variable)==NULL);
+  pre(m.getSpecies(variable)==NULL && m.getParameter(variable)==NULL);
   msg = "A <diffusionCoefficient>";
   if (dc.isSetId()) {
     msg += " with the id '" + dc.getId() + "'";
   }
-  msg += " has a value of '" + variable + "' for its 'variable', but the model does not contain a <species> with that id.";
+  msg += " has a value of '" + variable + "' for its 'variable', but the model does not contain a <species> or <parameter> with that id.";
 
   inv(false);
 }
 END_CONSTRAINT
 
 
+// 1223458
+START_CONSTRAINT(SpatialDiffusionCoefficientVariableMustNotBeSelf, DiffusionCoefficient, dc)
+{
+    pre(dc.isSetVariable());
+    const SBase* parent = dc.getParentSBMLObject();
+    pre(parent != NULL);
+    pre(parent->getId() == dc.getVariable());
+
+    stringstream ss_msg;
+    ss_msg << "A <diffusionCoefficient>";
+    if (dc.isSetId())
+    {
+        ss_msg << " with id '" << dc.getId() << "'";
+    }
+    ss_msg << " references its parent parameter '" << dc.getVariable() << "'.";
+    msg = ss_msg.str();
+    inv(false);
+}
+END_CONSTRAINT
+
+
 // 1223606
 START_CONSTRAINT(SpatialBoundaryConditionCoordinateBoundaryMustBeBoundary, BoundaryCondition, bc)
 {

diff --git a/src/sbml/packages/spatial/validator/constraints/SpatialConsistencyConstraintsDeclared.cxx b/src/sbml/packages/spatial/validator/constraints/SpatialConsistencyConstraintsDeclared.cxx
@@ -76,7 +76,8 @@ addConstraint(new VConstraintDiffusionCoefficientSpatialDiffusionCoefficientOneC
 addConstraint(new VConstraintDiffusionCoefficientSpatialDiffusionCoefficientCoordinateReferenceDifference(*this));
 addConstraint(new VConstraintDiffusionCoefficientSpatialDiffusionCoefficientCoordinateReferenceNoYIn1D(*this));
 addConstraint(new VConstraintDiffusionCoefficientSpatialDiffusionCoefficientCoordinateReferenceNoZIn2D(*this));
-addConstraint(new VConstraintAdvectionCoefficientSpatialAdvectionCoefficientVariableMustBeSpecies(*this));
+addConstraint(new VConstraintAdvectionCoefficientSpatialAdvectionCoefficientVariableMustBeSpeciesOrParam(*this));
+addConstraint(new VConstraintAdvectionCoefficientSpatialAdvectionCoefficientVariableMustNotBeSelf(*this));
 addConstraint(new VConstraintBoundaryConditionSpatialBoundaryConditionVariableMustBeSpecies(*this));
 addConstraint(new VConstraintCoordinateComponentSpatialBoundaryMinLessThanMax(*this));
 addConstraint(new VConstraintParameterSpatialBoundaryMustBeConstant(*this));
@@ -133,7 +134,8 @@ addConstraint(new VConstraintAnalyticVolumeSpatialAnalyticVolumeDomainTypeMustBe
 addConstraint(new VConstraintParametricObjectSpatialParametricObjectDomainTypeMustBeDomainType(*this));
 addConstraint(new VConstraintCSGObjectSpatialCSGObjectDomainTypeMustBeDomainType(*this));
 addConstraint(new VConstraintBoundaryConditionSpatialBoundaryConditionBoundaryDomainTypeMustBeDomainType(*this));
-addConstraint(new VConstraintDiffusionCoefficientSpatialDiffusionCoefficientVariableMustBeSpecies(*this));
+addConstraint(new VConstraintDiffusionCoefficientSpatialDiffusionCoefficientVariableMustBeSpeciesOrParam(*this));
+addConstraint(new VConstraintDiffusionCoefficientSpatialDiffusionCoefficientVariableMustNotBeSelf(*this));
 addConstraint(new VConstraintBoundaryConditionSpatialBoundaryConditionCoordinateBoundaryMustBeBoundary(*this));
 addConstraint(new VConstraintGeometrySpatialOneGeometryDefinitionMustBeActive(*this));
 addConstraint(new VConstraintBoundaryConditionSpatialBoundaryConditionBoundaryDomainTypeOrCoordinateBoundary(*this));

diff --git a/...sbml/packages/spatial/validator/test/test-data/general-constraints/1223404-fail-01-02.xml b/...sbml/packages/spatial/validator/test/test-data/general-constraints/1223404-fail-01-02.xml
@@ -66,9 +66,21 @@
         <listOfSpecies>
             <species boundaryCondition="false" compartment="compartment" constant="false" hasOnlySubstanceUnits="false" id="species_1" spatial:isSpatial="true"/>
         </listOfSpecies>
+        <listOfReactions>
+          <reaction id="J0" reversible="true" fast="false" spatial:isLocal="false">
+            <listOfReactants>
+              <speciesReference species="species_1" stoichiometry="1" constant="true"/>
+            </listOfReactants>
+            <kineticLaw>
+              <math xmlns="http://www.w3.org/1998/Math/MathML">
+                <cn type="integer"> 2 </cn>
+              </math>
+            </kineticLaw>
+          </reaction>
+        </listOfReactions>
         <listOfParameters>
             <parameter constant="false" id="id_3">
-                <spatial:diffusionCoefficient spatial:type="isotropic" spatial:variable="id_3"/>
+                <spatial:diffusionCoefficient spatial:type="isotropic" spatial:variable="J0"/>
             </parameter>
         </listOfParameters>
     </model>

diff --git a/...sbml/packages/spatial/validator/test/test-data/general-constraints/1223404-pass-00-02.xml b/...sbml/packages/spatial/validator/test/test-data/general-constraints/1223404-pass-00-02.xml
@@ -0,0 +1,77 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<sbml level="3" spatial:required="true" version="1" xmlns="http://www.sbml.org/sbml/level3/version1/core" xmlns:spatial="http://www.sbml.org/sbml/level3/version1/spatial/version1">
+    <model>
+        <spatial:geometry spatial:coordinateSystem="cartesian" spatial:id="geometry_1">
+            <spatial:listOfCoordinateComponents>
+                <spatial:coordinateComponent spatial:id="coordinateComponent_1" spatial:type="cartesianX" spatial:unit="dimensionless">
+                    <spatial:boundaryMin spatial:id="boundary_1" spatial:value="0"/>
+                    <spatial:boundaryMax spatial:id="boundary_2" spatial:value="1"/>
+                </spatial:coordinateComponent>
+                <spatial:coordinateComponent spatial:id="coordinateComponent_2" spatial:type="cartesianY" spatial:unit="dimensionless">
+                    <spatial:boundaryMin spatial:id="boundary_3" spatial:value="0"/>
+                    <spatial:boundaryMax spatial:id="boundary_4" spatial:value="1"/>
+                </spatial:coordinateComponent>
+                <spatial:coordinateComponent spatial:id="coordinateComponent_3" spatial:type="cartesianZ" spatial:unit="dimensionless">
+                    <spatial:boundaryMin spatial:id="boundary_5" spatial:value="0"/>
+                    <spatial:boundaryMax spatial:id="boundary_6" spatial:value="1"/>
+                </spatial:coordinateComponent>
+            </spatial:listOfCoordinateComponents>
+            <spatial:listOfDomainTypes>
+                <spatial:domainType spatial:id="domainType_1" spatial:spatialDimensions="3"/>
+            </spatial:listOfDomainTypes>
+            <spatial:listOfDomains>
+                <spatial:domain spatial:domainType="domainType_1" spatial:id="domain_1">
+                    <spatial:listOfInteriorPoints>
+                        <spatial:interiorPoint spatial:coord1="0" spatial:coord2="0" spatial:coord3="0"/>
+                    </spatial:listOfInteriorPoints>
+                </spatial:domain>
+            </spatial:listOfDomains>
+
+
+
+            <spatial:listOfGeometryDefinitions>
+                <spatial:analyticGeometry spatial:id="geometryDefinition_1" spatial:isActive="true">
+                    <spatial:listOfAnalyticVolumes>
+                        <spatial:analyticVolume spatial:domainType="domainType_1" spatial:functionType="layered" spatial:id="analyticVolume_1" spatial:ordinal="0">
+                            <math xmlns="http://www.w3.org/1998/Math/MathML"/>
+                        </spatial:analyticVolume>
+                    </spatial:listOfAnalyticVolumes>
+                </spatial:analyticGeometry>
+                <spatial:sampledFieldGeometry spatial:id="geometryDefinition_2" spatial:isActive="true" spatial:sampledField="sampledField_1">
+                    <spatial:listOfSampledVolumes>
+                        <spatial:sampledVolume spatial:domainType="domainType_1" spatial:id="sampledVolume_1" spatial:maxValue="0" spatial:minValue="0"/>
+                    </spatial:listOfSampledVolumes>
+                </spatial:sampledFieldGeometry>
+                <spatial:parametricGeometry spatial:id="geometryDefinition_3" spatial:isActive="true">
+                    <spatial:spatialPoints spatial:arrayDataLength="0" spatial:compression="uncompressed" spatial:dataType="double" spatial:id="spatialPoints_1"/>
+                    <spatial:listOfParametricObjects>
+                        <spatial:parametricObject spatial:compression="uncompressed" spatial:dataType="uint" spatial:domainType="domainType_1" spatial:id="parametricObject_1" spatial:pointIndexLength="0" spatial:polygonType="triangle"/>
+                    </spatial:listOfParametricObjects>
+                </spatial:parametricGeometry>
+
+
+
+
+
+            </spatial:listOfGeometryDefinitions>
+            <spatial:listOfSampledFields>
+                <spatial:sampledField spatial:compression="uncompressed" spatial:dataType="double" spatial:id="sampledField_1" spatial:interpolationType="nearestNeighbor" spatial:numSamples1="0" spatial:numSamples2="0" spatial:numSamples3="0" spatial:samplesLength="0"/>
+            </spatial:listOfSampledFields>
+        </spatial:geometry>
+        <listOfCompartments>
+            <compartment constant="false" id="compartment">
+                <spatial:compartmentMapping spatial:domainType="domainType_1" spatial:id="compartmentMapping_1" spatial:unitSize="1"/>
+            </compartment>
+        </listOfCompartments>
+        <listOfSpecies>
+            <species boundaryCondition="false" compartment="compartment" constant="false" hasOnlySubstanceUnits="false" id="species_1" spatial:isSpatial="true"/>
+        </listOfSpecies>
+        <listOfParameters>
+            <parameter id="param1" value="3" constant="false"/>
+            <parameter constant="false" id="id_3">
+                <spatial:diffusionCoefficient spatial:type="isotropic" spatial:variable="param1"/>
+            </parameter>
+        </listOfParameters>
+    </model>
+</sbml>
+