Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

timeOffset should be set for each simulation within a workflow #313

Closed
sfrechen opened this issue Aug 28, 2020 · 11 comments
Closed

timeOffset should be set for each simulation within a workflow #313

sfrechen opened this issue Aug 28, 2020 · 11 comments

Comments

@sfrechen
Copy link
Member

As specified in #18 :
We need to define an optional timeOffset for each simulation within a workflow. Default should be 0.
Both the simulation and connected data should be shifted by the timeOffset, i.e.
simulationTime - timeOffset

This is needed if the reference simulation differs in the study design compared to the simulations to be compared.

@Yuri05 Yuri05 added this to the Milestone 3 milestone Aug 28, 2020
@Yuri05
Copy link
Member

Yuri05 commented Aug 28, 2020

not for RE 1.0

@Yuri05
Copy link
Member

Yuri05 commented Apr 30, 2021

@sfrechen @TWendl

Both the simulation and connected data should be shifted by the timeOffset

  • Is this option required for both MeanModel and for Population workflows?
  • The shift should be applied only to the TimeProfile plots, correct?
  • How first/last application range plots should be handled in case of shifts?
  • ... (depending on the answers above I would have some more questions )

@sfrechen
Copy link
Member Author

sfrechen commented May 3, 2021

@Yuri05

Is this option required for both MeanModel and for Population workflows?

Yes.

The shift should be applied only to the TimeProfile plots, correct?

No, probably for PK parameter calculation as well. Implementation to be discussed.

How first/last application range plots should be handled in case of shifts?

  • The typical application for the timeOffset are DDI simulations with single dose administrations, i.e. nothing to do here, everything is simply shifted...

  • To generalize it for the multiple dose case WITH dose administrations happening AFTER the timeOffset , again nothing to do, everything is simply shifted...

  • To generalize it for the multiple dose case WITH dose administrations happening BEFORE the timeOffset (which is quite an unrealistic scenario!!), I would suggest to define the first application range as the first administration after the timeOffset and throw a warning...

@Yuri05
Copy link
Member

Yuri05 commented May 3, 2021

One possibility to implement this would be: instead of shifting the simulation results:
a) shift the start times of all applications and events by the offset BEFORE executing a simulation AND
b) shift all simulation output intervals by the time offset (also before executing) (if the upper bound of a shifted interval is <=0 - remove the interval; if the shifted upper bound is >0 but the shifted lower bound is <=0 - set the lower bound to 0)
c) shift the observed data by the time offset

Not sure if this is better/easier to implement - to be discussed. Advantage: PK calculation and all tasks (time profiles etc) know nothing about the shift and behave exactly as before.
Disadvantage: if a simulation contains any time dependent inputs beyond applications and events
(e.g. a parameter with af ormula explicitly depending on TIME) - simulation results might become wrong.

@msevestre msevestre added this to Accepted in RE Version 1.2 via automation Sep 3, 2021
@msevestre msevestre removed this from Accepted in RE Version 1.2 Sep 3, 2021
@msevestre msevestre added this to To do in Version 2 via automation Sep 3, 2021
@pchelle
Copy link
Collaborator

pchelle commented Oct 20, 2021

@sfrechen , can you let me know if this what is expected from the timeOffset option ?
In the sample,

  • blue was the reference population with timeOffset=0
  • red was from the other simulationSet and had a timeOffset=-2 (so, values are shifted 2h later, a value of 2 would shift 2h earlier but from the discussion above, if timeOffset>application time, the application should be excluded, so this would not work on this example)

Larson 2013 8y-18y 400mg FCT meal-timeProfile-Concentration-total

@pchelle
Copy link
Collaborator

pchelle commented Oct 20, 2021

To generalize it for the multiple dose case WITH dose administrations happening BEFORE the timeOffset (which is quite an unrealistic scenario!!), I would suggest to define the first application range as the first administration after the timeOffset and throw a warning

Here is a sample for multiple applications with no timeOffset
Monkey IV-timeProfile-Concentration-total

With timeOffset=2, the workflow will

  • throw the following warning: 1 applications at 0h happened before 'timeOffset' (2h) and were excluded from the time profile analysis
  • draw for the following total range
    Shifted Monkey IV-timeProfile-Concentration-total
  • draw the following first application range
    Shifted Monkey IV-timeProfile-Concentration-firstApplication

@sfrechen
Copy link
Member Author

sfrechen commented Oct 28, 2021

@sfrechen , can you let me know if this what is expected from the timeOffset option ? In the sample,

  • blue was the reference population with timeOffset=0
  • red was from the other simulationSet and had a timeOffset=-2 (so, values are shifted 2h later, a value of 2 would shift 2h earlier but from the discussion above, if timeOffset>application time, the application should be excluded, so this would not work on this example)

Yes. This looks correct to me. It is a but hard to judge wrt. observed data (since the names do not match with the simulations and are generally pretty off ;-). Are they also shifted?

With timeOffset=2, the workflow will

throw the following warning: 1 applications at 0h happened before 'timeOffset' (2h) and were excluded from the time > > profile analysis
draw for the following total range

The total range looks also correct. I just do not understand the warning 1 applications at 0h happened before 'timeOffset' (2h) and were excluded from the time profile analysis. The application is part of the plot and not excluded (which is as I would like to have it.)

draw the following first application range

I think here the upper warning would make sense and the plot should not be generated.

@Yuri05
Copy link
Member

Yuri05 commented Oct 28, 2021

I think here the upper warning would make sense and the plot should not be generated.

should we really don't generate the first app plot in this case?
@sfrechen

@sfrechen
Copy link
Member Author

I think here the upper warning would make sense and the plot should not be generated.

should we really don't generate the first app plot in this case? @sfrechen

Yes. I would not plot administrations for which application time < timeOffset

@pchelle
Copy link
Collaborator

pchelle commented Oct 29, 2021

Yes. This looks correct to me. It is a but hard to judge wrt. observed data (since the names do not match with the simulations and are generally pretty off ;-). Are they also shifted?

The observed data are also shifted.

@pchelle
Copy link
Collaborator

pchelle commented Oct 29, 2021

The total range looks also correct. I just do not understand the warning 1 applications at 0h happened before 'timeOffset' (2h) and were excluded from the time profile analysis. The application is part of the plot and not excluded (which is as I would like to have it.)

Then, I'll update the warning with only 1 applications at 0h happened before 'timeOffset' (2h).

I should be able to prevent the plot to be output in such a case.
@sfrechen , just for being thorough, what should happen if

  • Only one timeOffset for reference or compared population has application time before ? (warning + no plot at all ?)
  • If has only 1 application or select only the first application time range, while time offset is wrong ? (warning + no plot or error directly ?)

@Yuri05 Yuri05 moved this from To do to In progress in Version 2 Dec 10, 2021
pchelle added a commit to pchelle/OSPSuite.ReportingEngine that referenced this issue Dec 22, 2021
…n set for shifting time

While fixing this issue, I imported ospsuite.utils in NAMESPACE so ospsuite.utils:: should not be necessary any longer
@Yuri05 Yuri05 closed this as completed in 9776ad2 Jan 6, 2022
Version 2 automation moved this from In progress to Done Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

4 participants