Fixes #2359

[msevestre]

Fixes #2359

Description

Implement In Children criteria

Type of change

Please mark relevant options with an x in the brackets.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires documentation changes (link at least one user or developer documentation issue):
• Algorithm update - updates algorithm documentation/questions/answers etc.
• Other (please describe):

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Integration tests
• Unit tests
• Manual tests
• No tests required

Reviewer checklist

Mark everything that needs to be checked before merging the PR.

• Check if the code is well documented
• Check if the behavior is what is expected
• Check if the code is well tested
• Check if the code is readable and well formatted
• Additional checks (document below if any)
• Check if documentation update issue(s) are created if the option This change requires a documentation update above is selected

Screenshots (if appropriate):

Questions (if appropriate):

[msevestre]

took me a while to figure this one out. We have some caching going on that was outside of the loop. In my implementation, I decided to add a dynamic tag to exclude specific children only (e.g. the direct children) of the container

[msevestre]

tha'ts the gist of the logic. I create a tag, add it to all direct children and modify the criteria of the sum formula to exclude the tag explicitrely. I do not know how to do it otherwise

[Yuri05]

This means that in a simulation, all direct children entities will have some dummy tags? hmm
no side effects with this after e.g. update/commit or during comparison of the simulation with BBs or so?
The tags will be also e.g. added to the simulation text report (not a big problem, but strange).

[msevestre]

@PavelBal Please check this out. I think this is what you meant but I am not sure :)

[PavelBal]

I think this is what I meant, lol :D Looks good. @msevestre

[rwmcintosh]

Just a couple of formatting adjustments to make

[Yuri05]

After looking at the implementation, I'm not sure if it's correct or if I just misunderstand the requirement.
@PavelBal Is this new "in children" container criteria meant as "in all DIRECT children of a container"?

So with your example from the issue: should it sum up only the volume of the organs (Bone|Volume+Brain|Volume+...) but not the volume of organ compartments (like Bone|Plasma|Volume etc.)?

Use case - a parameter Volume of the container Organism should sum up all Volume parameters of the childer of Organism. Right now, we require an additional tag assigned to all children containers of "Organism".

@Yuri05 The requirement says same as IN container but excluding the parent container. So it should be in ALL sub containers of the parent recursively no?

[msevestre]

well in fact good question @Yuri05
if the idea is to get only the DIRECT container, then this is wrong.

[PavelBal]

Hmm, I actually meant in ALL CHILDREN.

But I didn't think about

So with your example from the issue: should it sum up only the volume of the organs (Bone|Volume+Brain|Volume+...) but not the volume of organ compartments (like Bone|Plasma|Volume etc.)?

I guess we can keep it as "in all children".

[Yuri05]

so the current implementation only takes the DIRECT children into account?
I'm lost...

[msevestre]

hum no

//root
         //  |Container1
         //  |  |Dynamic = sum of all volume in children of Container 1
         //  |  |volume = 4 (this parameter should not be used)
         //  |  |SubContainer1_1
         //  |  |  |volume = 1
         //  |  |SubContainer1_2
         //  |  |  |volume = 2
         //  |Container2
         //  |  |SubContainer2_2
         //  |  |  |volume = 3 (this parameter should not be used)

the parameter is defined in Container 1
so it takes SubContainer1_1 and SubContainer1_2 but not from the parent parent.
I don't have a two level check here but this is the way it would work

[msevestre]

it just happens that the test does nto have SubSubContainers. I can add if you want

[msevestre]

and I thought that a diagram would make everything super clear lol.

[Yuri05]

ok, I think the implementation is correct :)
If there are no side effects with adding a new tag during the simulation creation - we can merge.

[msevestre]

If… lol

do you see any other way to exclude dynamically?
All tests are passing. When we merge in PKSim, we should see

[Yuri05]

do you see any other way to exclude dynamically?

not at the moment :)