Fixes #2260 - including "Global" PK-Parameters into the box-plot/scatter plot #2359
Conversation
| @@ -59,7 +59,7 @@ private PopulationAnalysisPKParameterField createFieldFrom(QuantityPKParameter p | |||
| { | |||
| var quantity = _allQuantities[pkParameter.QuantityPath]; | |||
| if (quantity == null) | |||
| return null; | |||
| return _populationAnalysisFieldFactory.CreateFor(pkParameter, QuantityType.Drug | QuantityType.Observer, quantityDisplayPath); | |||
There was a problem hiding this comment.
I'm not sure here. Clearly the quantity is not going to be found in the _allQuantities cache. But I'm not sure how get around this. Should there be additional values added int _allQuantities?
There was a problem hiding this comment.
I think this is ok but this requires a comment explaining that this parameter is not found because it is a global PK Parameter (can we actually check if this is one?). It cil;d also be the the quantity was removed somehow (thinking about mobi models imported etc..)
| } | ||
|
|
||
| private GlobalPKAnalysis calculateGlobalPKAnalysisFor(Simulation simulation) | ||
| private IContainer calculateGlobalPKAnalysisFor(Simulation simulation, Compound compound, DataColumn peripheralVenousBloodCurve, DataColumn venousBloodCurve, PKCalculationOptions options) |
There was a problem hiding this comment.
This method is now called per compound so that it was easier to do the body weight scaling. The content should be the same otherwise.
| @@ -29,7 +30,7 @@ public abstract class concern_for_GlobalPKAnalysisTask : ContextSpecification<IG | |||
| private IVSSCalculator _vssCalculator; | |||
| protected DataColumn _peripheralVenousBloodPlasma; | |||
| protected DataColumn _venousBloodPlasma; | |||
| protected IndividualSimulation _simulation; | |||
There was a problem hiding this comment.
I refactored this test a bit because the base abstract class was setting up everything for an individual simulation, then the population_based was subclassing that and adding some population simulations. When I wrote my tests, I couldn't figure out why they were failing. The assertions were using the wrong simulation!
| return medianAggregateDataColumns(VenousBloodColumns(compoundName)); | ||
| } | ||
|
|
||
| public IReadOnlyList<DataColumn> VenousBloodColumns(string compoundName) |
| return medianAggregateDataColumns(PeripheralVenousBloodColumns(compoundName)); | ||
| } | ||
|
|
||
| public IReadOnlyList<DataColumn> PeripheralVenousBloodColumns(string compoundName) |
There was a problem hiding this comment.
GlobalPKAnalysisTask needs to get access to all the columns for PeripheralVenousBlood and VenousBlood
| @@ -20,7 +20,7 @@ protected override void Context() | |||
| public class When_checking_if_the_simulation_comparison_tree_node_context_menu_factory_can_create_a_context_menu_for_a_given_node : concern_for_SimulationComparisonTreeNodeContextMenuFactory | |||
| { | |||
| [Observation] | |||
| public void shouldu_return_true_if_the_node_is_a_classifiable_comparision_node() | |||
| public void should_return_true_if_the_node_is_a_classifiable_comparision_node() | |||
There was a problem hiding this comment.
still a typo lol comparision => comaprison
There was a problem hiding this comment.
There are a few instances of that. I'll fix them all, but one is in a database query that I won't change.
migrateTo5_3
| private IReadOnlyList<DataColumn> drugColumnFor(string organ, string compartment, string columnName, string compoundName) | ||
| private DataColumn drugColumnForIndividual(string organ, string compartment, string columnName, string compoundName, int individualId) | ||
| { | ||
| return columnsFor(Results.IndividualResultsAsArray().FirstOrDefault(result => result.IndividualId == individualId), organ, compartment, columnName, compoundName); |
There was a problem hiding this comment.
There has to be away to access results by Id other than this? This looks very odd to me/.Plus the array conversion etc ..
There was a problem hiding this comment.
Results.ResultsFor(individualId) :) Nice.
| @@ -330,5 +337,15 @@ private DataColumn columnsFor(IndividualResults results, string organ, string co | |||
| Values = column.Values | |||
| }; | |||
| } | |||
|
|
|||
| public DataColumn VenousBloodColumnForIndividual(int individualId, string compoundName) | |||
There was a problem hiding this comment.
do they still need to be public?
There was a problem hiding this comment.
We are using that from PKAnalysesTask, or what do you mean? Do we need this to be a datacolumn at all? And can we work from the results directly?
There was a problem hiding this comment.
just wondering if we need tthis sutff outside of the class. But we do. I wasn't sure
| globalPKAnalysis.Add(calculateGlobalPKParameters(populationSimulation, individualId, compoundName, options, globalPKAnalysis)); | ||
| }); | ||
|
|
||
| mapQuantityPKParametersFromIndividualGlobalPKAnalyses(globalPKAnalysesForIndividuals.ToList()).Each(x => analyses.AddPKAnalysis(x)); |
There was a problem hiding this comment.
why the ToList. It's already an enumerable
globalPKAnalysesForIndividuals.Each(analyses.AddPKAnalysis)
|
|
||
| var simulation = allSimulations[0]; | ||
|
|
||
| var globalPKAnalysis = new GlobalPKAnalysis(); |
There was a problem hiding this comment.
initialize right at the beginnign and return on line 155. Otherwise, you are sometimes initializing sometime returning. Not consistent (*with line 162)
| var peripheralVenousBloodColumn = simulation.PeripheralVenousBloodColumnForIndividual(individualId, moleculeName); | ||
| var venousBloodColumn = simulation.VenousBloodColumnForIndividual(individualId, moleculeName); | ||
|
|
||
| return calculateGlobalPKAnalysisFor(simulation, simulation.Compounds.FirstOrDefault(x => string.Equals(x.Name, moleculeName)), peripheralVenousBloodColumn, venousBloodColumn, calculationOptions); |
There was a problem hiding this comment.
FindByName or even maybe a method CompoundByName r sthg? but the first one for sure we have as ExtMethod
|
|
||
| private static GlobalPKAnalysis globalPkAnalysisForIndividual(ICache<int, GlobalPKAnalysis> globalPKAnalysesForIndividuals, int individualId) | ||
| { | ||
| GlobalPKAnalysis globalPKAnalysis; |
There was a problem hiding this comment.
There is something unpleasant about this code. It is a mix of two approaches
I would suggest to do either
1
if (globalPKAnalysesForIndividuals.Keys.Contains(individualId))
return globalPKAnalysesForIndividuals[individualId];
globalPKAnalysesForIndividuals[individualId] = new GlobalPKAnalysis();;
return globalPKAnalysesForIndividuals[individualId];
or
if (!globalPKAnalysesForIndividuals.Keys.Contains(individualId))
globalPKAnalysesForIndividuals[individualId] = new GlobalPKAnalysis();;
return globalPKAnalysesForIndividuals[individualId];
| // use the first in series as a template to retrieve from all individual results. | ||
| // The list of parameters should be identical for all the individual global analyses. | ||
| var aPKAnalysis = globalIndividualPKParameterList.FirstOrDefault(); | ||
| aPKAnalysis?.AllPKParameters.GroupBy(pkParameter => pkParameter.EntityPath().ToPathArray().MoleculeName()).Each(group => |
There was a problem hiding this comment.
pkParameter.EntityPath().ToPathArray().MoleculeName()
That's ugly too. Let's create a method somewhere to extract MoleculeName for a pkParameter . There has to be something doing it somewhere too
| private static QuantityPKParameter quantityPKParameterFor(IReadOnlyList<GlobalPKAnalysis> globalIndividualPKParameterList, IParameter pKParameter, string quantityPath) | ||
| { | ||
| var pKValuesForPKParameter = globalIndividualPKParameterList.SelectMany(globalPKAnalysisForAnIndividual => globalPKAnalysisForAnIndividual.AllPKParameters.Where(x => pathsEqual(x, pKParameter))).ToList(); | ||
| var quantityPKParameter = new QuantityPKParameter { Dimension = pKValuesForPKParameter.First().Dimension, Name = pKValuesForPKParameter.First().Name, QuantityPath = quantityPath }; |
There was a problem hiding this comment.
are we sure here that there will be at least one? (first)
| _entityPathResolver = entityPathResolver; | ||
| } | ||
|
|
||
| public void UpdateBodyWeightForIndividual(PopulationSimulation populationSimulation, int individualId) |
There was a problem hiding this comment.
Is there a reason why this was moved out of the PK Stuff? Because you are doing line 24 to 26 now for EAch individual as opposed to one time.
There was a problem hiding this comment.
The only reason for it was that originally I was reproducing the scaling logic in a second location. I think it makes sense to put it back now.
| @@ -69,6 +69,16 @@ private void createAvailableQuantitiesListBasedOn(IPopulationDataCollector popul | |||
| { | |||
| _allPKParameters.Clear(); | |||
| _allPKParameters.AddRange(_entitiesInContainerRetriever.OutputsFrom(populationDataCollector).SelectMany(availablePKParametersFor)); | |||
|
|
|||
| populationDataCollector.Compounds.Select(x => x.Name).Each(compoundName => | |||
| @@ -59,7 +59,7 @@ private PopulationAnalysisPKParameterField createFieldFrom(QuantityPKParameter p | |||
| { | |||
| var quantity = _allQuantities[pkParameter.QuantityPath]; | |||
| if (quantity == null) | |||
| return null; | |||
| return _populationAnalysisFieldFactory.CreateFor(pkParameter, QuantityType.Drug | QuantityType.Observer, quantityDisplayPath); | |||
There was a problem hiding this comment.
I think this is ok but this requires a comment explaining that this parameter is not found because it is a global PK Parameter (can we actually check if this is one?). It cil;d also be the the quantity was removed somehow (thinking about mobi models imported etc..)
msevestre
deleted the
2260-include-global-pk-parameters-in-pop-analyses
branch
No description provided.