Fixes #248 solubility as table #610

msevestre · 2018-04-18T00:12:14Z

No description provided.

…ed as table or not

msevestre · 2018-04-18T00:13:36Z

@Yuri05 Big pull request that should basically support Table solubility in PK-Sim
There is a test that creates a simulation using Table Solubility. This test WILL fail as long as the implementation is not supported in SimModel.

SimModelNET.SimModelException : Formula type 'TableFormulaWithXArgument' is unknown

msevestre · 2018-04-18T00:14:01Z

src/PKSim.Assets/PKSimConstants.cs

@@ -2235,7 +2239,7 @@ public static string ImportFormulationDescription
            {
               var sb = new StringBuilder();
               sb.AppendLine("The Excel table for formulation import should have a format of two colums.");
-               sb.AppendLine("The first column represents the Time (unit Time e.g. in years).");
+               sb.AppendLine("The first column represents the Time (unit Time e.g. in hours).");


Kind of wrong :)

msevestre · 2018-04-18T00:14:34Z

src/PKSim.Core/Model/CompoundFactory.cs

@@ -51,7 +54,6 @@ private void initializeCalculationMethods(Compound compound)
      private void readCompoundFromTemplate(Compound compound)
      {
         //STEP1: Add all parameters defined for the compound from the database
-         //need to set the name of the compound to drug in order to load parameter from database


as always, comments are lying

msevestre · 2018-04-18T00:14:53Z

src/PKSim.Core/Model/FormulaFactory.cs

@@ -311,6 +311,9 @@ private IFormula createFormula(RateKey rateKey)
         if (rateKey.IsTableWithOffsetFormula)


This should theoretically not be required once we upgrade the code

might be required for old projects (unless we don't convert old TableWithOffset formulas into TableWithXArg)

No this is only used when creating a new Formula using the RateKey (e..g from DB).
That should never happen anymore. TableWithOffset will still be available as an object for backward compatibility

msevestre · 2018-04-18T00:16:56Z

src/PKSim.Core/Services/ICompoundAlternativeTask.cs

+      /// Performs initalization steps for a brand new solubility alternative that should behave as a Table alternative
+      /// </summary>
+      /// <param name="solubilityAlternative"></param>
+      void PrepareSolubilityAlternativeForTableSolubility(ParameterAlternative solubilityAlternative);


I created a public method for that because the same logic needs to be called when loading from snapshot. Not ideal at the moment but works until something better falls into place

msevestre · 2018-04-18T00:17:37Z

src/PKSim.Infrastructure/ProjectConverter/v7_3/Converter721To730.cs

+         simulation.Compounds.Each(convertCompound);
+      }
+
+      private void convertCompound(Compound compound)


Project conversion also implemented. Simply adds a clone of the default Table parameter

msevestre · 2018-04-18T00:18:30Z

src/PKSim.Presentation/Presenters/Parameters/EditTableParameterPresenter.cs

@@ -49,16 +51,21 @@ public bool Edit(IParameter tableParameter)
      private void tableFormulaChanged(object sender, EventArgs eventArgs)
      {
         plotTable();
+         View.OkEnabled = CanClose;


IT was possible to close the table edit without any points. This create a table that is not suppored by schema

msevestre · 2018-04-18T00:18:43Z

src/PKSim.Presentation/Presenters/Parameters/TableParameterPresenter.cs

+            if (_editedFormula == null)
+               return base.CanClose;
+
+            return base.CanClose && _editedFormula.AllPoints().Any();


Can only close if there is at least one point

msevestre · 2018-04-18T00:19:21Z

src/PKSim.Presentation/Services/FullPathDisplayResolver.cs

+            case IQuantity quantity:
+               return _quantityDisplayPathMapper.DisplayPathAsStringFor(quantity, addSimulationName);
+            case ParameterAlternativeGroup parameterAlternativeGroup:
+               return displayFor(parameterAlternativeGroup);


That was also a little bug in the way we display a parameter alternative in the command description

msevestre · 2018-04-18T00:20:08Z

src/PKSim.UI/Views/Parameters/MultiParameterEditView.cs

@@ -149,7 +149,6 @@ public void BindTo(IEnumerable<ParameterDTO> parameters)
         //necessary to bind to binding list to enable automatic update from value since we are 
         //using the autobind property from devexpress which only (!!) reacts to IBindingList properties.
         _gridViewBinder.BindToSource(parameters.ToBindingList());
-         gridView.BestFitColumns();


This was removed to ensure that the ValueOrigin column does not take a crazy amount of space

msevestre · 2018-04-18T00:32:52Z

@Yuri05
Table is exported as follow:

 "UseDerivedValues": true,
        "Points": [
          {
            "X": 5.0,
            "Y": 22.0,
            "RestartSolver": false
          },
          {
            "X": 10.0,
            "Y": 50.0,
            "RestartSolver": false
          }

Is this correct (I do not know what UseDerivedValues means anymore)

Yuri05 · 2018-04-18T08:31:06Z

Is this correct

No, must be "UseDerivedValues": false
Otherwise dY/dX (here: dSolubility/dpH) will be used in the simulation

Yuri05 · 2018-04-18T10:35:22Z

src/PKSim.Assets/PKSimConstants.cs

@@ -2157,6 +2160,7 @@ public static class UI
         public static readonly string UseWatermark = "Use watermark in charts when exporting to clipboard?";
         public static readonly string WatermarkText = "Text";
         public static readonly string WatermarkProperties = "Watermark Properties";
+         public static readonly string CreateTableSolubilityAlternative = "Create as pH-Sol table";



Think Create as pH-Solubility table would be better

:)

changed

msevestre · 2018-04-18T00:58:28Z

src/PKSim.Infrastructure/Services/MoleculeBuilderFactory.cs

            _parameterSetUpdater.UpdateValue(alternativeParameter, drugParameter);
+
+            //Default parameter Default and visible may not match database default and need to be set according to alternative parameter
+            drugParameter.IsDefault = alternativeParameter.IsDefault;


@Yuri05 This is important here to ensure that parameter such as RefPh, SolubilityAtRefPh etc.. do not appear as changed. By default they are IsDefault= false. But in this case, they need to inherit from the compound value

msevestre · 2018-04-18T12:33:07Z

src/PKSim.Assets/PKSimConstants.cs

@@ -2157,6 +2160,7 @@ public static class UI
         public static readonly string UseWatermark = "Use watermark in charts when exporting to clipboard?";
         public static readonly string WatermarkText = "Text";
         public static readonly string WatermarkProperties = "Watermark Properties";
+         public static readonly string CreateTableSolubilityAlternative = "Create as pH-Sol table";



:)

changed

msevestre · 2018-04-18T12:34:12Z

src/PKSim.Core/Model/FormulaFactory.cs

@@ -311,6 +311,9 @@ private IFormula createFormula(RateKey rateKey)
         if (rateKey.IsTableWithOffsetFormula)


No this is only used when creating a new Formula using the RateKey (e..g from DB).
That should never happen anymore. TableWithOffset will still be available as an object for backward compatibility

msevestre · 2018-04-18T12:34:24Z

src/PKSim.Core/Model/FormulaFactory.cs

+
+         return formula;
+      }
+
      private IFormula createTableFormulaWithOffset(RateKey rateKey)


All of that should go to

…for solubility table import

msevestre · 2018-04-18T13:27:12Z

src/PKSim.Assets/PKSimConstants.cs

@@ -2157,6 +2160,7 @@ public static class UI
         public static readonly string UseWatermark = "Use watermark in charts when exporting to clipboard?";
         public static readonly string WatermarkText = "Text";
         public static readonly string WatermarkProperties = "Watermark Properties";
+         public static readonly string CreateTableSolubilityAlternative = "Create as pH-Sol table";



msevestre · 2018-04-18T13:27:24Z

src/PKSim.Assets/PKSimConstants.cs

-
+         }
+
+         public static string ImportSolubilityTableDescription


Added a description too

msevestre · 2018-04-18T13:27:32Z

src/PKSim.Assets/PKSimConstants.cs

@@ -2238,13 +2238,25 @@ public static string ImportFormulationDescription
            get
            {
               var sb = new StringBuilder();
-               sb.AppendLine("The Excel table for formulation import should have a format of two colums.");


this was not not english

msevestre · 2018-04-18T13:27:56Z

src/PKSim.Core/Model/FormulaFactory.cs

@@ -260,9 +261,10 @@ public IFormula RateFor(RateKey rateKey, IFormulaCache formulaCache)
         return formulaCache[rateKey];
      }

-      public TableFormula CreateTableFormula()
+      public TableFormula CreateTableFormula(bool useDerivedValues = true)


add the parameter. Default is true (same behavior as previous)

msevestre · 2018-04-18T13:28:03Z

src/PKSim.Core/Model/ParameterAlternativeFactory.cs

@@ -59,11 +59,10 @@ public ParameterAlternative CreateTableAlternativeFor(ParameterAlternativeGroup
         var alternative = CreateAlternativeFor(compoundParameterGroup);

         var tableParameter = alternative.Parameter(tableParameterName);
-         var tableFormula = _formulaFactory.CreateTableFormula();
+         var tableFormula = _formulaFactory.CreateTableFormula(useDerivedValues:false);


creates with false in this case

msevestre added 10 commits April 6, 2018 09:32

Rename caption for import formulation

0d2d106

Add special view with check box to define if solubility should be add…

ca6d69f

…ed as table or not

Add first implementation of import solubility table

68ed130

Small refactoring for ValuePointDTO

9fa66ea

WIP Open-Systems-Pharmacology#248 solubility table

810187b

Start implementation of solubility in DB

d63eafe

Update code to use TableFormulaWithXArgument from core

2e449ec

Add tests and project conversion

933d56a

Add integration test for solubility table used in simulation

f6afc7d

Fixes Open-Systems-Pharmacology#248 solubility as table

435c7fd

msevestre requested a review from Yuri05 April 18, 2018 00:12

msevestre changed the title ~~248 solubility as table~~ Fixes #248 solubility as table Apr 18, 2018

msevestre commented Apr 18, 2018

View reviewed changes

msevestre added the prio: high label Apr 18, 2018

msevestre added 2 commits April 17, 2018 20:23

Some updates for PR

28ebc51

Update XRef to XArg

785104a

Ensure that visible and IsDefault flag matches alternative parameter

7bf6979

Yuri05 reviewed Apr 18, 2018

View reviewed changes

msevestre commented Apr 18, 2018

View reviewed changes

Update according to pull request. Also adds tool tip and description …

201804e

…for solubility table import

msevestre commented Apr 18, 2018

View reviewed changes

Yuri05 approved these changes Apr 18, 2018

View reviewed changes

Yuri05 merged commit 574a77a into Open-Systems-Pharmacology:develop Apr 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #248 solubility as table #610

Fixes #248 solubility as table #610

msevestre commented Apr 18, 2018

msevestre commented Apr 18, 2018 •

edited

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

Yuri05 Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre commented Apr 18, 2018

Yuri05 commented Apr 18, 2018

Yuri05 Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

msevestre Apr 18, 2018

		@@ -311,6 +311,9 @@ private IFormula createFormula(RateKey rateKey)
		if (rateKey.IsTableWithOffsetFormula)

Fixes #248 solubility as table #610

Fixes #248 solubility as table #610

Conversation

msevestre commented Apr 18, 2018

msevestre commented Apr 18, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msevestre commented Apr 18, 2018

Yuri05 commented Apr 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msevestre commented Apr 18, 2018 •

edited