add update_density and update_parameter function

[elmbeech]

pull request for the staled pull request #264, #250 and #232.

this pull request is compatible with physicell version 1.14.0 and keeps the changes to the existing code bases as small as possible.
default settings were chosen so that the current models not will break through these changes.
each case was manually tested, the code works as expected.

add_density and add_parameter functions:

Microenvironment::add_density: throw error, if a density with the same name already exists.
Parameters::add_parameter : throw error, if a parameter with the same name and type already exists.

update_density and update_parameter functions:

Microenvironment::update_density : new function which does not throw an error but updates the values, if a density with the same name already exists.
Parameters::update_parameter : new function which does not throw an error but updates the values, if a parameter with the same name and type already exists.

function calls arguments:

load_PhysiCell_config_file: additional bool update_variables argument with default set to false.
setup_microenvironment_from_XML (called from load_PhysiCell_config_file): additional bool update_density argument with default set to false.
User_Parameters::read_from_pugixml (called from load_PhysiCell_config_file): additional bool update_parameter argument with default set to false.

[elmbeech]

as discussed, update_density and update_parameter through an error, if one tries to update a density or parameter that not already exists. the rest is the same.

[drbergman]

Can you call the above implementation of update_parameter to simplify this function and the codebase?

[elmbeech]

@drbergman i am not sure if i really can do what you ask for, as i try to update parameters.
i don't have "default values", as in the add case.
this is as concise as i can do it.
please write here how i really should do it so that it is acceptable for you. thank you.

[drbergman]

My intention is something super simple and it seems you took a big step towards it having all 3 of these call assert_exists. But something like this could make it even tighter:

void Parameters<T>::update_parameter( std::string my_name , T my_value , std::string my_units )
{
	update_parameters( my_name, my_value); // will check my_name exists and then set the value to my_value
	parameters[find_index(my_name)].units = my_units;
	return;
}

[drbergman]

or I think you could even use parameters[my_name] instead of parameters[find_index(my_name)]

[drbergman]

also, looking at the [] operator on strings, it already checks for existence.

[drbergman]

oh yeah? I mean, we could throw in some const and pass by reference to reduce any overhead costs. I imagine these functions won't be called nearly enough though to make a noticeable impact on performance. Or will they?

Finally, I personally find this kind of coding to be so much easier to maintain that it easily makes it worth the computational tradeoff (at least as I imagine the magnitude of the cost)

[elmbeech]

is this ok?

[drbergman]

hmmm...I don't think I'm able to communicate this...your original is the best of these, in my opinion

[elmbeech]

oh yes, it will get called often. I my case every iteration.
but i know, the PhysiCell core code is not written for only one project. so this argument has no weight.
at this moment i just try to get the code, the new functions, into the base. that's all.

[elmbeech]

@drbergman could you please confirm that the conflict / conversation / change you requested has resolved (as i think so ).
or let me know if otherwise?

i don't think it is appropriate when i click the resolved button.

thank you, Elmar

[drbergman]

Can you use the above implementation of update_density to simplify this function and the codebase?

[elmbeech]

is this ok?

[drbergman]

yup

[elmbeech]

i still think it is odd to have to implement a slower version because of code maintenance.
before we had to call the find_index function 1 time, now 3 or 4 times.
just to avoid storing the value in a variable?

[elmbeech]

this is the compromise i suggest between maintenance (as closed as it get to the existing add_parameter function) and speed.
i hope this is acceptable.

[elmbeech]

and the same for the densities:

[elmbeech]

i pushed these changes and hope they are ok and get merged.

[elmbeech]

@drbergman could you please confirm that the conflict / conversation / change you requested has resolved (as i think so ).
or let me know if otherwise?

i don't think it is appropriate when i click the resolved button.

thank you, Elmar

[elmbeech]

@drbergman can you please give your ok to this change?
I really would like to get this pull request in now!
thank you, Elmar