WIP #486 expose data repository #487

msevestre · 2021-03-18T23:49:19Z

This is just a WIP. I would like to understand what we need exactly
So far

library(ospsuite)

obsData <- loadObservedData("tests/data/obs_data.pkml")

> obsData$baseGrid
  Time: [min]

> obsData$columns
[[1]]
  Time: [min]

[[2]]
  Measurement: [kg/l]

[[3]]
  Error: [kg/l]

> obsData$columns[[2]]$values
[1] 1.01e-09 8.30e-10 7.30e-10 2.80e-10 1.20e-10 3.05e-11 5.93e-12

> col<-obsData$columns[[2]]
> col$dimension
[1] "Concentration (mass)"
> col$displayUnit
[1] "mg/l"
> col$unit
[1] "kg/l"
> col$name
[1] "Measurement"
> col$values
[1] 1.01e-09 8.30e-10 7.30e-10 2.80e-10 1.20e-10 3.05e-11 5.93e-12

msevestre

WIP

msevestre · 2021-03-18T23:53:43Z

R/data-repository.R

+      DataColumn$new(private$wrapReadOnlyProperty("BaseGrid", value))
+    },
+    #' @field columns Returns all columns (including baseGrid defined in the data)
+    columns = function(value){


@PavelBal @Yuri05 I will cache this so that we always return the same instances of DataColumn wrapper

msevestre · 2021-03-18T23:55:15Z

R/data-repository.R

+  cloneable = FALSE,
+  active = list(
+    #' @field baseGrid Returns the base column for the population (typically time column)
+    baseGrid = function(value) {


in core, we have basegrid, columns, and allButBaseGrid.
I will implement the same methods (baseGrid is typically time, columns are all columns in the dataRepo, allButBaseGrids are all except basegrid)

Should we have a property called Time that returned baseGrid? or property called Measurement that returns col2 and a property called error that returns col3? not sure

Should we have a property called Time that returned baseGrid? or property called Measurement that returns col2 and a property called error that returns col3?

I would say no and no. Basegrid might be different from "Time", we could have several measurements and several errors, so I wouldn't provide shortcuts for the specific use case "ObservedDataRepo = {Time, Measurement, Error}"

we could have several measurements and several errors,

Reallly, do we have it somewhere (and how)? @Yuri05

In theory results from a simulation etc.. are DataRepositories
So yes we can have one time and multiple measurements. Each measurements can in theory have multiple errors. I think we are using this for the PI results.
Practically probably not

R/data-column.R

PavelBal

@msevestre I propose some changes to the naming.

R/data-repository.R

PavelBal · 2021-03-22T15:12:09Z

R/data-repository.R

+  cloneable = FALSE,
+  active = list(
+    #' @field baseGrid Returns the base column for the population (typically time column)
+    baseGrid = function(value) {


we could have several measurements and several errors,

Reallly, do we have it somewhere (and how)? @Yuri05

R/data-repository.R

R/utilities-observed-data.R

msevestre · 2021-03-22T18:56:42Z

R/data-repository.R

+    },
+    #' @field metaData Returns a named list of meta data defined for the data repository.
+    #' where the name is the name of the metaData and the value is the meta data value.
+    metaData = function(value){


@PavelBal @Yuri05
Meta data is a named list of {name, value}

> metaData $Source [1] "C:\\temp\\RanorexTestData\\ObservedData.xlsx" $Sheet [1] "Tabelle1" $DoubleValue [1] 5 $StringValues [1] "hello" $IntegerValue [1] 4

msevestre · 2021-03-22T18:57:28Z

R/data-repository.R

+        if (is.null(private$.metaData)) {
+          netExtendedProperties <- private$wrapReadOnlyProperty("ExtendedProperties", value)
+          netMetaData <- rClr::clrGet(netExtendedProperties, "All")
+          names <- unlist(lapply(netMetaData,  function(data) rClr::clrGet(data, "Name")))


@PavelBal Any way to do this 3 lines in one? Seems to be a bit stupid. At any rate, the array will be very small anyways so I don't mind iterating twice

Can't think of anything.

msevestre · 2021-03-22T21:39:32Z

R/init-package.R

@@ -18,6 +18,4 @@ initPackage <- function() {
  apiConfig$pkParametersFilePath <- filePathFor("OSPSuite.PKParameters.xml")

  rClr::clrCallStatic("OSPSuite.R.Api", "InitializeOnce", apiConfig$ref)
-  dimensionTask <- getNetTask("DimensionTask")


@Yuri05 THis was creating some issues. And then I wondered. Do we actually need to load if from .NET
isn't it enough to have it a constant as it is now?

msevestre · 2021-03-22T21:40:25Z

R/utilities.R

@@ -48,6 +48,10 @@ ifNotNull <- function(condition, outputIfNotNull, outputIfNull = NULL) {
 #' See https://github.com/Open-Systems-Pharmacology/OSPSuite-R/issues/476 for details
 #' @import stringr
 encodeUnit <- function(unit) {
+
+  #TODO maybe we don't even need to load from .NET
+  # dimensionTask <- getNetTask("DimensionTask")


@Yuri05 If we want to load from .NET we would need to do it here the first time is is being used. But why do we need this in the first place since ospsuiteEnv$muSymbol is already set
see ospsuite-env.R

Specifies the symbol used for µ. This will be set by the .NET layer

ospsuiteEnv$muSymbol <- "µ"

PavelBal · 2021-03-24T15:42:53Z

R/data-repository.R

+        if (is.null(private$.metaData)) {
+          netExtendedProperties <- private$wrapReadOnlyProperty("ExtendedProperties", value)
+          netMetaData <- rClr::clrGet(netExtendedProperties, "All")
+          names <- unlist(lapply(netMetaData,  function(data) rClr::clrGet(data, "Name")))


unlist(x, use.names = FALSE) is much faster than unlist(x).

No idea what the actual benefit would be here, but why not add it..

PavelBal · 2021-03-24T15:49:06Z

R/init-package.R

@@ -18,6 +18,4 @@ initPackage <- function() {
  apiConfig$pkParametersFilePath <- filePathFor("OSPSuite.PKParameters.xml")

  rClr::clrCallStatic("OSPSuite.R.Api", "InitializeOnce", apiConfig$ref)
-  dimensionTask <- getNetTask("DimensionTask")


Why do you remove "DimensionTask"? I was counting on it for the utilities-dimensions.

Btw. I only was creating it the first time it was requested:

In ospsuite-env (esqlabsEnv in my code):

# NetTask "DimensionTask" cached for performance benefits. Created the first time it is requested. esqlabsEnv$DimensionTask <- NULL

Then a function somewhere:

#' Return an instance of the .NET Task "DimensionTask". #' #' @return An instance of the Task getDimensionTask <- function() { dimTask <- esqlabsEnv$DimensionTask if (is.null(dimTask)) { dimTask <- getNetTask("DimensionTask") esqlabsEnv$DimensionTask <- dimTask } return(dimTask) }

I don't remove it. Instatntiating object in the init creates weird error messages during build.

Can you quantify cached for performance benefits..

I think you are looking at sthg else. :)

but I am going to add it to core because it's probably a good optimization since this is used quite often

PavelBal · 2021-03-24T15:50:26Z

R/utilities.R

@@ -48,6 +48,10 @@ ifNotNull <- function(condition, outputIfNotNull, outputIfNull = NULL) {
 #' See https://github.com/Open-Systems-Pharmacology/OSPSuite-R/issues/476 for details
 #' @import stringr
 encodeUnit <- function(unit) {
+
+  #TODO maybe we don't even need to load from .NET


Oh you have it here... See my suggestion above.

I don't understand what you mean

WIP #486 expose data repository

6d58e21

msevestre requested review from PavelBal and Yuri05 March 18, 2021 23:49

msevestre commented Mar 18, 2021

View reviewed changes

msevestre added 3 commits March 18, 2021 20:05

Cache columns

55eca27

Ensures that the AllButBaseGrid method is a get

6d7a020

WIP #486 expose data repository

062c2ab

PavelBal reviewed Mar 22, 2021

View reviewed changes

R/data-column.R Show resolved Hide resolved

PavelBal requested changes Mar 22, 2021

View reviewed changes

msevestre added 6 commits March 22, 2021 14:07

Fixes #486 expose data repository

caa0e6f

Fixes #486 expose data repository

f5a1a6e

Update test file to support multiple data type

63823cc

Update example

f76f2af

Update example

98e6822

Fixes #488

5fb2635

msevestre commented Mar 22, 2021

View reviewed changes

PavelBal reviewed Mar 24, 2021

View reviewed changes

PavelBal requested changes Mar 24, 2021

View reviewed changes

Fixes #486 expose data repository

937ae0f

msevestre merged commit 04c0fce into develop Mar 24, 2021

msevestre deleted the 486-expose-data-repository branch March 24, 2021 16:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP #486 expose data repository #487

WIP #486 expose data repository #487

msevestre commented Mar 18, 2021 •

edited

Loading

msevestre left a comment

msevestre Mar 18, 2021

msevestre Mar 18, 2021

Yuri05 Mar 19, 2021

PavelBal Mar 22, 2021

msevestre Mar 22, 2021

PavelBal left a comment

PavelBal Mar 22, 2021

msevestre Mar 22, 2021

msevestre Mar 22, 2021

PavelBal Mar 24, 2021

msevestre Mar 22, 2021

msevestre Mar 22, 2021

PavelBal Mar 24, 2021

msevestre Mar 24, 2021

PavelBal Mar 24, 2021

msevestre Mar 24, 2021

msevestre Mar 25, 2021

msevestre Mar 25, 2021

PavelBal Mar 24, 2021

msevestre Mar 24, 2021

WIP #486 expose data repository #487

WIP #486 expose data repository #487

Conversation

msevestre commented Mar 18, 2021 • edited Loading

msevestre left a 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

PavelBal left a 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

Specifies the symbol used for µ. This will be set by the .NET layer

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 Mar 18, 2021 •

edited

Loading