Throw error if duplicate substrate or parameter

[drbergman]

Check that each added substrate has a unique name. Same for parameters (for a given type).

Also, clean up some code to improve maintainability/readability.

Suggestion for future improvement: rename some of the many instances of "parameters" in the code. For example, UserParameters has one instance called parameters. Each instance of Parameters has a member called parameters. Many other objects in PhysiCell have parameters members. This makes it hard to track which ones belong where.

[elmbeech]

@drbergman, @heberlr, @MathCancer I am still voting for not thronging an Error, maybe not even a Warning.

The reason is, that:
Parameters and densities are high-level variables of PhysiCell and should, in my opinion, be handled accordingly.

If these are variables, then the add_parameter and add_density is the same as when we e.g. in Python write:

a = 1
a = 2

If we later on assign another value to a variable, Python will not through an error, complain that "a" already exists. Because it is a variable and not a constant, python will simply overwrite the variable content.

In the same, in my opinion, should PhysiCell handle add_variable and add_parameter.
This means, we only have to make sure that the variables are not generated twice, if someone two times calls the add_parameter or add_density function. Nothing else. Without error and warning, but checking for the existence and not generating the variable if it already exists we handle this gracefully, without unnecessary breaking!@

[elmbeech]

If you, for whatever reason, insist that add_parameter and add_density through an error, then I request that at least the following:

• in the PhysiCell_settings.cpp / setup_microenvironment_from_XML function, the code should check if the density already exists, before add_density is called, and jump over it if the density already exists and sets the values (units, diffusion_constant, decay_rate).

• in PhysiCell_settinngs.cpp / User_Parameters::read_from_pugixml function, the code should check if the parameter already exists, before the add_parameter is called, and jump over it if the parameter already exists and set the values (value, units).

This would solve my problem too. But I think throwing no Error and no Waring is what we actually should do.

[drbergman]

These suggestions would walk back the main purpose of this PR: don't let users define a substrate (or parameter) and quietly not use it because it turned out to be a duplicate. We discussed the reasons for this change previously (#232). If you want to work towards a solution that fits within the constraints of this PR, I would suggest the following:

1. if this is your own project: just use the code you have now. it's ok if a project uses a variation on PhysiCell
2. if this is a tool for others to use: optional arguments that a user could overwrite to change the error -> warning within add_density and add_parameter. Then, the basic main.cpp can remain unchanged but users who want to downgrade the error to a warning can do so in main.cpp.

[elmbeech]

@drbergman, the main purpose of the original pull request #232 (which actually was from me and not from you) was that parameters and densities variables can be reloaded without eating up memory, by every time generating a new variable with the same name.
we discussed the reason, but the real issue why this pull request was generated in the first hand is not resolved and was not really address in the discussion because the people who were discussing it were not really understanding the problem.

this pull request is here for a tool for other to use, and this is why it is critical that is properly resolved in the main code base and my "python example" above, that I later on came up with because I tried to deal with the new situation, should make quite clear what the real problem is.

[drbergman]

We discussed this over Zoom and in #232. I sided with you in #232. The rest of the community though agreed that this should result in an error. Heber had clear points on this in that thread. You asked for help amending that PR and that's why I made this one.

You have not acknowledged the issue we are trying to prevent with throwing an error, and that makes it difficult to continue this conversation. I have tried to ask why you need to loop through "episodes" in your main.cpp rather than running multiple instances and you declined to answer. So, to the extent I do not understand the problem, it is because you are withholding that information. I offered a possible solution above. Did you look into that? I'll offer another potential solution: move the load_PhysiCell_config_file call before your episodic loop. Since you have a single config file for all your episodes, you should not need to load the config file multiple times.

As for your python example, add_density clearly suggests adding a density to the list of current densities, i.e., not allowing for an overwrite. If the function were add_or_update_density, then that would properly indicate that the function could just update the density if it already existed.

I'm going to pause my own activity on this thread until others weigh in.

[elmbeech]

ok, i have now a version with add and update possibility and a flag that is by default set to add.
this should now make everyone happy.
have still fully to check that all works fine and think this is best tomorrow morning with a fresh mind. i should have a proper pull request by tomorrow evening.

[elmbeech]

@drbergman I made a pull request with my changes to this branch on your PhysiCell fork.

[elmbeech]

this pull request is superseded by pull request #264 .

this version with add_density, add_parameter, update_density, and update_parameter functions should now make everyone happy.
for more details, please read the comment in the #264 pr.

thank you, Elmar

[drbergman]

Great! I'll take a look once the checks are all passing on #264.

[elmbeech]

Thank you, @drbergman !

I looks like github has at this moment in general problem to run MacOSX jobs.
They are never picked up and executed.
I try to track down what the problem is. If it is based on our action or a problem at github it self.
After it is fixed, could you please re-run the jobs? I can't because I have no write permissions in the MathCancer PhysiCell repo.

• https://docs.github.com/en/actions/managing-workflow-runs/re-running-workflows-and-jobs

Thank you, Elmar

[elmbeech]

@vincent-noel and @drbergman
It looks that looks like macosx 11 was removed for github actions. We have to switch to 12!

[elmbeech]

@vincent-noel

This is the line that have to be changed in the yaml files:

runs-on: macos-11

Will try to figure out if there is something like macos-latest.

I will change it in my pull request.
That would fix it for anyone, if accepted ; ).

[elmbeech]

@drbergman , i just have a chat with @vincent-noel !
pull request #242 will fix the problem.
as soon this pull request is accepted, the macosx check should pass too.

thank you.

[elmbeech]

So, @drbergman feel free to have a look at the pull request #264 already now ; ).