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

Define VolumePlasma as SumFormula #2118

Closed
msevestre opened this issue Mar 16, 2022 · 14 comments
Closed

Define VolumePlasma as SumFormula #2118

msevestre opened this issue Mar 16, 2022 · 14 comments
Assignees

Comments

@msevestre
Copy link
Member

see Open-Systems-Pharmacology/Forum#1174 (comment)

@Yuri05 Do you know why it's still a hard coded formula?
How could we defined it dynamically? Using Plasma as a tag I guess? Thoughts?

@msevestre
Copy link
Member Author

/cc @PavelBal

@PavelBal
Copy link
Member

Yes, I was surprised that it is still a hard-coded sum.

I have implemented this in my models with the following criteria:

image

@msevestre
Copy link
Member Author

and what about the rest of the formula ?

@PavelBal
Copy link
Member

The default:

image

@Yuri05
Copy link
Member

Yuri05 commented Mar 16, 2022

I wonder why the condition is "In Container" Plasma and not "Tagged with" Plasma?

@StephanSchaller
Copy link
Member

I thought we implemented this a long time ago (last year) based on our specifications.. I remember marco albrecht sending a document specifying all left-over hard-coded sums as a sum formula... anyway...

@msevestre
Copy link
Member Author

Better late than never

@PavelBal
Copy link
Member

I wonder why the condition is "In Container" Plasma and not "Tagged with" Plasma?

Could you explain the main differences between "Tagged" and "In Container" and why you think "Tagged with" would be more appropriate here?

@Yuri05
Copy link
Member

Yuri05 commented Mar 17, 2022

"Tagged" = container itself.
"In container" = container itself or any direct or indirect parent container

Now imagine you define in Mobi a new subcontainer of e.g. Bone/Plasma and reduce the Bone/Plasma/Volume by the volume of this new subcontainer.
"Tagged with 'Plasma'" will return for Bone Bone/Plasma/Volume - as expected
"In container 'Plasma'" will return for Bone Bone/Plasma/Volume + Bone/Plasma/Subcontainer/Volume - wrong

@PavelBal
Copy link
Member

Oh, you are right then. Didn't think about the children of the container.

@msevestre Must be "Tagget with: Plasma" then.

@msevestre msevestre self-assigned this Mar 22, 2022
@msevestre
Copy link
Member Author

So it does not work with Taged with "Plasma"
We are looking for Parameter Volume. If we tag them with "Plasma", we need to add the Tag "Plasma" to all parameters.
This is what we do with TissueOrgan (WHY?!? @Yuri05 )

image

Do we add the tag of the parent container to all parameters?

@msevestre
Copy link
Member Author

@Yuri05
yes
https://github.com/Open-Systems-Pharmacology/PK-Sim/blob/develop/src/PKSim.Core/Model/PKSimSpatialStructureFactory.cs#L90

We are adding the tag of the parent container to all children. that's why the "TAGGED TissueOrgan" works. I don't think this is a good way of doing things. But of course, example above would create a problem (that's why we have to remove Periportal and Pericentral)

@PavelBal
Copy link
Member

I don't think this is a good way of doing things.

I agree, I do not like this at all. I actually think that "In Container: Plasma" is a better solution. For Yuris use case - well the user would have to take care of manually excluding the sub-compartment.

@Yuri05
Copy link
Member

Yuri05 commented Mar 23, 2022

We are adding the tag of the parent container to all children. that's why the "TAGGED TissueOrgan" works. I don't think this is a good way of doing things. But of course, example above would create a problem (that's why we have to remove Periportal and Pericentral)

Yep. I think all this comes from the time before (NOT) In Container criteria were introduced. That's why we have this for parameters and also for containers:
grafik

Ideally we should go through all our formulas and replace (NOT) Match condition with (NOT) In Container condition where possible (=where it refers to any parent container in the hierarchy of the object); and when done for all formulas - remove all those unnecessary "parent containers" tags.
But would take some time and not high prio for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants