WIP Fixes failign build #625
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,26 +14,36 @@ namespace MoBi.Engine.Sbml | |
| { | ||
| internal class CombinedUnit | ||
| { | ||
| public string Name => (_direct?.Name ?? "1") + (_inverse != null ? $"/{_inverse.Name}" : ""); | ||
|
|
||
| // Rate is the factor to convert from the unit used in sbml to the base unit used in sbml. | ||
| // This is not necessarily the base unit in the ospsuite (1/s vs 1/min for inversed day for instance) | ||
| public double Rate { get; private set; } = 1; | ||
|
|
||
| private Unit _direct; | ||
| private Unit _inverse; | ||
|
|
||
| public void AddUnit(int kind, double exponent, double multiplier, double scale, IDictionary<int, Unit> baseUnitsDictionary) | ||
| { | ||
| if (!baseUnitsDictionary.ContainsKey(kind)) | ||
| return; | ||
|
|
||
| if (exponent < 0) | ||
| _inverse = baseUnitsDictionary[kind]; | ||
| else | ||
| _direct = baseUnitsDictionary[kind]; | ||
|
|
||
| Rate *= multiplier * Math.Pow(10, scale); | ||
| } | ||
|
There was a problem hiding this comment. private menber. Why a get set? There was a problem hiding this comment. Not really needed, sometimes I do this for finding easier where it is been used. So even when it is private, it is a full property and you can easily navigate but you are right, not necessary. There was a problem hiding this comment. Resharper navigates with private members...plus it follows the rest of the code |
||
| } | ||
|
|
||
| internal class UnitConversionInfo | ||
| { | ||
| public IDimension Dimension { get; set; } | ||
|
|
||
| //Unit in which the values are stored in SBML Base unit | ||
| public Unit Unit { get; set; } | ||
|
There was a problem hiding this comment. @abdelr Comment here? Is this correct? |
||
|
|
||
| public double Rate { get; set; } | ||
| } | ||
|
|
||
|
|
@@ -48,37 +58,39 @@ public class UnitDefinitionImporter : SBMLImporter, IStartable, IUnitDefinitionI | |
| { | ||
| private readonly IMoBiDimensionFactory _moBiDimensionFactory; | ||
| private readonly IDictionary<int, Unit> _baseUnitsDictionary; | ||
|
There was a problem hiding this comment. why this guy here sinec we have a MoBiDimensionFactory (which is a specification of IDimensionFActory) |
||
| private readonly IDictionary<string, UnitConversionInfo> _unitConversionDictionary; | ||
|
|
||
| public IReadOnlyDictionary<string, IDimension> ConversionDictionary => _unitConversionDictionary.ToDictionary(kv => kv.Key, kv => kv.Value.Dimension); | ||
|
|
||
| private readonly IDictionary<string, string> _sbmlUnitsSynonyms = new Dictionary<string, string>() | ||
| { | ||
| {"litre", "l"} | ||
| }; | ||
|
|
||
| public UnitDefinitionImporter(IObjectPathFactory objectPathFactory, IObjectBaseFactory objectBaseFactory, IMoBiDimensionFactory mobiDimensionFactory, ASTHandler astHandler, IMoBiContext context) : | ||
| base(objectPathFactory, objectBaseFactory, astHandler, context) | ||
| { | ||
| _moBiDimensionFactory = mobiDimensionFactory; | ||
| _baseUnitsDictionary = new Dictionary<int, Unit>(); | ||
| _unitConversionDictionary = new Dictionary<string, UnitConversionInfo>(); | ||
| initBaseUnitsDictionary(); | ||
| } | ||
|
|
||
| public string TranslateUnit(string sbmlUnit) | ||
| { | ||
| if (_sbmlUnitsSynonyms.ContainsKey(sbmlUnit)) | ||
| return _sbmlUnitsSynonyms[sbmlUnit]; | ||
|
|
||
| return sbmlUnit; | ||
| } | ||
|
|
||
| private void initBaseUnitsDictionary() | ||
| { | ||
| _baseUnitsDictionary.Add(libsbml.UNIT_KIND_AMPERE, _moBiDimensionFactory.Dimension("Ampere").Unit("A")); | ||
| _baseUnitsDictionary.Add(libsbml.UNIT_KIND_BECQUEREL, _moBiDimensionFactory.Dimension("Becquerel").Unit("Bq")); | ||
| _baseUnitsDictionary.Add(libsbml.UNIT_KIND_CANDELA, _moBiDimensionFactory.Dimension("Candela").Unit("cd")); | ||
| _baseUnitsDictionary.Add(libsbml.UNIT_KIND_COULOMB, _moBiDimensionFactory.Dimension("Coulomb").Unit("C")); | ||
| _baseUnitsDictionary.Add(libsbml.UNIT_KIND_DIMENSIONLESS, _moBiDimensionFactory.NoDimension.BaseUnit); | ||
|
There was a problem hiding this comment. I am pretty sure you mean BaseUnit and not DefaultUnit. We always need to convert to BaseUnit. |
||
| _baseUnitsDictionary.Add(libsbml.UNIT_KIND_FARAD, _moBiDimensionFactory.Dimension("Becquerel").Unit("Bq")); | ||
| _baseUnitsDictionary.Add(libsbml.UNIT_KIND_GRAM, _moBiDimensionFactory.Dimension("Mass").Unit("g")); | ||
| _baseUnitsDictionary.Add(libsbml.UNIT_KIND_GRAY, _moBiDimensionFactory.Dimension("Gray").Unit("Gy")); | ||
|
|
@@ -111,7 +123,9 @@ private void InitBaseUnitsDictionary() | |
|
|
||
| protected override void Import(Model model) | ||
| { | ||
| if (model == null) | ||
| return; | ||
|
|
||
| if (_sbmlProject == null) return; | ||
|
|
||
| for (long i = 0; i < model.getNumUnitDefinitions(); i++) | ||
|
|
@@ -121,7 +135,7 @@ protected override void Import(Model model) | |
| } | ||
|
|
||
| /// <summary> | ||
| /// Converts a SBML UnitDefinition into a MoBi Unit. | ||
| /// </summary> | ||
| public IDimension ConvertUnit(UnitDefinition unitDefinition) | ||
| { | ||
|
|
@@ -131,7 +145,7 @@ public IDimension ConvertUnit(UnitDefinition unitDefinition) | |
| if (dimension != Constants.Dimension.NO_DIMENSION) | ||
| { | ||
| _sbmlInformation.MobiDimension[sbmlUnit] = dimension; | ||
| _unitConversionDictionary.Add(sbmlUnit, new UnitConversionInfo {Dimension = dimension, Unit = dimension.BaseUnit, Rate = 1}); | ||
|
There was a problem hiding this comment. I am pretty sure you mean BaseUnit and not DefaultUnit. We always need to convert to BaseUnit. |
||
| return dimension; | ||
| } | ||
|
|
||
|
|
@@ -146,45 +160,48 @@ public IDimension ConvertUnit(UnitDefinition unitDefinition) | |
| sbmlUnitDefinition.getScale(), | ||
| _baseUnitsDictionary); | ||
| } | ||
|
|
||
| var unitName = combinedUnit.Name; | ||
| var dimensionAndUnit = _moBiDimensionFactory.FindUnit(unitName); | ||
| if (dimensionAndUnit.dimension != Constants.Dimension.NO_DIMENSION) | ||
| { | ||
| _sbmlInformation.MobiDimension[sbmlUnit] = dimensionAndUnit.dimension; | ||
| _unitConversionDictionary.Add(sbmlUnit, new UnitConversionInfo {Dimension = dimensionAndUnit.dimension, Unit = dimensionAndUnit.unit, Rate = combinedUnit.Rate}); | ||
| return dimensionAndUnit.dimension; | ||
| } | ||
|
|
||
| _unitConversionDictionary.Add(sbmlUnit, new UnitConversionInfo {Dimension = dimension, Unit = dimension.BaseUnit, Rate = 1}); | ||
|
There was a problem hiding this comment. I am pretty sure you mean BaseUnit and not DefaultUnit. We always need to convert to BaseUnit. |
||
| return dimension; | ||
| } | ||
|
|
||
| public override void AddToProject() | ||
| { | ||
| } | ||
|
|
||
| public IDimension DimensionFor(string sbmlUnit) | ||
| { | ||
| if (_unitConversionDictionary.ContainsKey(sbmlUnit)) | ||
| return _unitConversionDictionary[sbmlUnit].Dimension; | ||
|
|
||
| return _moBiDimensionFactory.DimensionForUnit(TranslateUnit(sbmlUnit)) ?? _moBiDimensionFactory.NoDimension; | ||
| } | ||
|
|
||
| public (double value, IDimension dimension) ToMobiBaseUnit(string unit, double value) | ||
| { | ||
| if (_unitConversionDictionary.ContainsKey(unit)) | ||
| { | ||
| var conversionData = _unitConversionDictionary[unit]; | ||
| return (conversionData.Dimension.UnitValueToBaseUnitValue(conversionData.Unit, value * conversionData.Rate), conversionData.Dimension); | ||
|
There was a problem hiding this comment. Why do we need this rate? It feels that yo are converting twice since we are also converting using the value There was a problem hiding this comment. @msevestre There are indeed two conversions. The first conversion is based on the description on the sbml file. The second is to the mobi base unit. Suppose there is a description from unit A to a (sbml) base unit B described in the file. This will be captured by the rate. Unit A does not exist on mobi but unit B does. Additionally, unit B needs to be translated to mobi base unit C. More specifically, we have an entry on the sbml file that specifies that "unit A" is going to be used and it has a given rule to be translated to "mole" (which is a conventional unit known in mobi too). This translation is captured in rate. But when we translate in mobi, we actually need to translate from "unit A" to "micro mole" which is the base unit, so we need to translate from "unit A" to "mole" and then to "micro mole". Does it make sense? It is not a clean explanation but we could talk about this tomorrow if you like. I will take a look on the changes though and also on the failing test. |
||
| } | ||
|
|
||
| var dimension = DimensionFor(unit); | ||
|
|
||
| return (dimension.UnitValueToBaseUnitValue(dimension.FindUnit(TranslateUnit(unit)), value), dimension); | ||
| } | ||
|
|
||
| public void Start() | ||
| { | ||
| _unitConversionDictionary.Clear(); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abdelr Comment here. Can you verify that this is correct?