avoid the possibility of having multiple parameters or multiple densities with the same name. #232

elmbeech · 2024-05-20T13:33:18Z

In the current PhysiCell implementation, it is possible to generate multiple parameters with the same name. It is also possible to multiple substrates (density) with the same name.

I changed the modules/PhysiCell_settings.cpp add_parameter functions so that they first check if a parameter with this name already exists, before pushing a new entry to the container.

I changed the BioFVM/BioFVM_microenvironment.cpp add_density functions so, that they first check if a density with this name already exist, before pushing a new entry to the containers.

This change will avoid having more than one parameter or density with the same name.

…e and multiple densities with the same name.

drbergman · 2024-06-03T12:33:56Z

This is a good improvement! Let's also take the opportunity to make this more maintainable and change the first two add_density implementations as follows:

void Microenvironment::add_density( void )
{
	// fix in PhysiCell preview November 2017 
	// default_microenvironment_options.use_oxygen_as_first_field = false; 
	return add_density( "unnamed" , "none" );
}

void Microenvironment::add_density( std::string name , std::string units )
{
	// fix in PhysiCell preview November 2017 
	// default_microenvironment_options.use_oxygen_as_first_field = false; 
	return add_density( name , units , 0.0 , 0.0 );
}

drbergman · 2024-06-03T13:56:49Z

BioFVM/BioFVM_microenvironment.cpp

+        }
+        else
+        {
+		std::cout << "Warning: density " << name << " alredy exist!" << std::endl;


I think we ought to throw an error here. I know that's not strictly backwards compatible , but we should force the user to reckon with adding two identical substrates.

For readability, let's have the current else block be the first case (if (find_density_index( name ) >= 0)) so that we can throw an error (or return if that's what we choose) and then do away with the brackets afterwards

MathCancer · 2024-06-03T19:44:04Z

I think this is a great idea, particularly in throwing an error in many cases.

It seems like this is still under active diswcusssion / development, so let's not approve just yet.

elmbeech · 2024-06-04T02:48:10Z

thank you @drbergman for the suggestions. I will look into it.

elmbeech · 2024-06-04T13:37:41Z

Hmm, @drbergman and @MathCancer, throwing an Error will cause troubles with the very project that made me aware of this issue.
In my project, during a run, I have to be able to reset parameters and densities to their initial values, if they already exist or generate a new parameter or density, if it does not already exist.
If we now through an Error, instead of a Warning, I gained nothing with this change.

I argue, so far, for running PhysiCell the common way, it was never a problem that it would have been possible to have multiple same named densities or parameters. So, spitting out a Waring and simply having the values overwritten should be good enough for the common user. There has not to be an Error.

drbergman · 2024-06-04T13:41:43Z

in that case, I think it's ok to leave it as a warning for now. it's how PhysiCell has operated all this time anyways.

heberlr · 2024-06-04T16:54:50Z

@elmbeech The framework cannot be designed project specific, if we think about the future and troubleshooting simulations, throwing a error should be more appropriate, as this can avoid potential issues in duplication in the grammar keys.

elmbeech · 2024-06-04T17:31:09Z

@heberlr in I do not agree.
it has to be possible to reset parameter and density structs to their initial values.
and with an error instead of a warning this becomes impossible with the current highlevel functions.

in any case, we talk here about a subject that was not a problem for the last 10 years at all.
now when I detected it and tried to fix it to solve my problem, you put another burden on me.

heberlr · 2024-06-04T18:43:48Z

Let's try to list the pros and cons:
Pros:

PhysiCell Studio already handles substrates with the same name. If you include substrates with the same name, the generated XML file has just one substrate.
User parameters are currently used by customizable C++ code. Should we expect the C++ developer to check the warnings?

Cons:

If you create two substrates with the same name in the .xml file and add rules associated with the substrate, the outcome won't be the expected one (throwing an error could help).
Example:
Using the template project, add this rule:
default,substrate,increases,apoptosis,0.1,0.5,10,0
Try running two simulation with two different microenvironment setups:

 <microenvironment_setup>
        <variable name="substrate" units="dimensionless" ID="0">
            <physical_parameter_set>
                <diffusion_coefficient units="micron^2/min">0.0</diffusion_coefficient>
                <decay_rate units="1/min">0.0</decay_rate>
            </physical_parameter_set>
            <initial_condition units="dimensionless">0</initial_condition>
            <Dirichlet_boundary_condition units="dimensionless" enabled="false">0</Dirichlet_boundary_condition>
            <Dirichlet_options>
                <boundary_value ID="xmin" enabled="True">0</boundary_value>
                <boundary_value ID="xmax" enabled="True">0</boundary_value>
                <boundary_value ID="ymin" enabled="True">0</boundary_value>
                <boundary_value ID="ymax" enabled="True">0</boundary_value>
                <boundary_value ID="zmin" enabled="False">0</boundary_value>
                <boundary_value ID="zmax" enabled="False">0</boundary_value>
            </Dirichlet_options>
        </variable>
        <variable name="substrate" units="dimensionless" ID="1">
            <physical_parameter_set>
                <diffusion_coefficient units="micron^2/min">1000.0</diffusion_coefficient>
                <decay_rate units="1/min">0.0</decay_rate>
            </physical_parameter_set>
            <initial_condition units="dimensionless">1</initial_condition>
            <Dirichlet_boundary_condition units="dimensionless" enabled="True">1</Dirichlet_boundary_condition>
            <Dirichlet_options>
                <boundary_value ID="xmin" enabled="True">1</boundary_value>
                <boundary_value ID="xmax" enabled="True">1</boundary_value>
                <boundary_value ID="ymin" enabled="True">1</boundary_value>
                <boundary_value ID="ymax" enabled="True">1</boundary_value>
                <boundary_value ID="zmin" enabled="False">1</boundary_value>
                <boundary_value ID="zmax" enabled="False">1</boundary_value>
            </Dirichlet_options>
        </variable>
        <options>
            <calculate_gradients>true</calculate_gradients>
            <track_internalized_substrates_in_each_agent>true</track_internalized_substrates_in_each_agent>
            <initial_condition type="matlab" enabled="false">
                <filename>./config/initial.mat</filename>
            </initial_condition>
            <dirichlet_nodes type="matlab" enabled="false">
                <filename>./config/dirichlet.mat</filename>
            </dirichlet_nodes>
        </options>
    </microenvironment_setup>

        <variable name="substrate" units="dimensionless" ID="0">
            <physical_parameter_set>
                <diffusion_coefficient units="micron^2/min">1000.0</diffusion_coefficient>
                <decay_rate units="1/min">0.0</decay_rate>
            </physical_parameter_set>
            <initial_condition units="dimensionless">1</initial_condition>
            <Dirichlet_boundary_condition units="dimensionless" enabled="True">1</Dirichlet_boundary_condition>
            <Dirichlet_options>
                <boundary_value ID="xmin" enabled="True">1</boundary_value>
                <boundary_value ID="xmax" enabled="True">1</boundary_value>
                <boundary_value ID="ymin" enabled="True">1</boundary_value>
                <boundary_value ID="ymax" enabled="True">1</boundary_value>
                <boundary_value ID="zmin" enabled="False">1</boundary_value>
                <boundary_value ID="zmax" enabled="False">1</boundary_value>
            </Dirichlet_options>
        </variable>
        <variable name="substrate" units="dimensionless" ID="1">
            <physical_parameter_set>
                <diffusion_coefficient units="micron^2/min">0.0</diffusion_coefficient>
                <decay_rate units="1/min">0.0</decay_rate>
            </physical_parameter_set>
            <initial_condition units="dimensionless">0</initial_condition>
            <Dirichlet_boundary_condition units="dimensionless" enabled="false">0</Dirichlet_boundary_condition>
            <Dirichlet_options>
                <boundary_value ID="xmin" enabled="True">0</boundary_value>
                <boundary_value ID="xmax" enabled="True">0</boundary_value>
                <boundary_value ID="ymin" enabled="True">0</boundary_value>
                <boundary_value ID="ymax" enabled="True">0</boundary_value>
                <boundary_value ID="zmin" enabled="False">0</boundary_value>
                <boundary_value ID="zmax" enabled="False">0</boundary_value>
            </Dirichlet_options>
        </variable>

The dilemma is: Should we help the user build the model and prevent unexpected behavior by not allowing duplicate names, or should we expect them to look for the outcomes by checking the warning messages?

One argument in favor of your PR is that nowadays the likelihood of an user developing a model by editing the XML is very low.

elmbeech · 2024-06-05T00:06:37Z

ok. you don't get the real problem. anyhow through an error. it's ok with me. i don't care.

elmbeech · 2024-06-05T15:45:40Z

superseded by pull request #250 . NF

drbergman · 2024-06-05T19:22:42Z

I think you mean #250. but yes, this one can be closed

avoid the possibility of having multiple parameters with the same nam…

f5cf2ce

…e and multiple densities with the same name.

drbergman reviewed Jun 3, 2024

View reviewed changes

elmbeech mentioned this pull request Jun 5, 2024

set default substrate color fn to give easy access to all projects #251

Merged

elmbeech closed this Jun 5, 2024

drbergman mentioned this pull request Jun 9, 2024

Throw error if duplicate substrate or parameter #250

Merged

elmbeech mentioned this pull request Jul 14, 2024

handle duplicate density and parameter #264

Closed

elmbeech deleted the variable_name_index branch September 17, 2024 17:46

elmbeech mentioned this pull request Sep 21, 2024

add update_density and update_parameter function #298

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

avoid the possibility of having multiple parameters or multiple densities with the same name. #232

avoid the possibility of having multiple parameters or multiple densities with the same name. #232

elmbeech commented May 20, 2024

drbergman commented Jun 3, 2024

drbergman Jun 3, 2024

MathCancer commented Jun 3, 2024

elmbeech commented Jun 4, 2024

elmbeech commented Jun 4, 2024 •

edited

Loading

drbergman commented Jun 4, 2024

heberlr commented Jun 4, 2024

elmbeech commented Jun 4, 2024 •

edited

Loading

heberlr commented Jun 4, 2024

elmbeech commented Jun 5, 2024 •

edited

Loading

elmbeech commented Jun 5, 2024 •

edited

Loading

drbergman commented Jun 5, 2024

avoid the possibility of having multiple parameters or multiple densities with the same name. #232

avoid the possibility of having multiple parameters or multiple densities with the same name. #232

Conversation

elmbeech commented May 20, 2024

drbergman commented Jun 3, 2024

drbergman Jun 3, 2024

Choose a reason for hiding this comment

MathCancer commented Jun 3, 2024

elmbeech commented Jun 4, 2024

elmbeech commented Jun 4, 2024 • edited Loading

drbergman commented Jun 4, 2024

heberlr commented Jun 4, 2024

elmbeech commented Jun 4, 2024 • edited Loading

heberlr commented Jun 4, 2024

elmbeech commented Jun 5, 2024 • edited Loading

elmbeech commented Jun 5, 2024 • edited Loading

drbergman commented Jun 5, 2024

elmbeech commented Jun 4, 2024 •

edited

Loading

elmbeech commented Jun 4, 2024 •

edited

Loading

elmbeech commented Jun 5, 2024 •

edited

Loading

elmbeech commented Jun 5, 2024 •

edited

Loading