diff --git a/addons/libRoadrunner/src/librr_intracellular.cpp b/addons/libRoadrunner/src/librr_intracellular.cpp index 1765c9532..e1096bab5 100644 --- a/addons/libRoadrunner/src/librr_intracellular.cpp +++ b/addons/libRoadrunner/src/librr_intracellular.cpp @@ -43,7 +43,14 @@ RoadRunnerIntracellular::RoadRunnerIntracellular(RoadRunnerIntracellular* copy) // initial_values = copy->initial_values; // mutations = copy->mutations; parameters = copy->parameters; - + // rrHandle and result are left at their in-class initializers; a RoadRunner instance + // cannot be copied, so clone() calls start() to create this one's own. +} + +RoadRunnerIntracellular::~RoadRunnerIntracellular() +{ + rrc::freeRRInstance( rrHandle ); + rrc::freeRRCData( result ); } // Parse the info in the .xml for (possibly) each , e.g. @@ -258,6 +265,18 @@ void RoadRunnerIntracellular::update() // return 0; } +// Fatal, like the setup-time check in validate_SBML_species(): there is no value to return +// and nothing sensible to write, so continuing would feed a fabricated number into the model. +static void librr_unknown_species_fatal(const std::string& species_name, const char* caller) +{ + std::cerr << std::endl + << "ERROR: " << caller << "() was asked for the SBML species \"" << species_name + << "\", which is not present in this model." << std::endl + << " Check the spelling against the species in the SBML file." << std::endl + << std::endl; + exit(-1); +} + double RoadRunnerIntracellular::get_parameter_value(std::string param_name) { rrc::RRVectorPtr vptr; @@ -277,14 +296,17 @@ double RoadRunnerIntracellular::get_parameter_value(std::string param_name) // std::string species_name = this->substrate_species[substrate_name]; // std::cout << " species_name = " << species_name << std::endl; - vptr = rrc::getFloatingSpeciesConcentrations(this->rrHandle); - //std::cerr << vptr->Count << std::endl; - for (int kdx=0; kdxCount; kdx++) + // find(), not operator[]: the inserting form silently yields column 0 for an unknown + // species, so a typo reads a different species than the caller named, and a read + // mutates the map. + std::map::const_iterator it = species_result_column_index.find( param_name ); + if( it == species_result_column_index.end() ) { - //std::cerr << kdx << ") " << vptr->Data[kdx] << std::endl; + librr_unknown_species_fatal( param_name, __FUNCTION__ ); } - int offset = species_result_column_index[param_name]; + vptr = rrc::getFloatingSpeciesConcentrations(this->rrHandle); + int offset = it->second; //std::cout << " result offset = "<< offset << std::endl; // double res = this->result->Data[offset]; double res = vptr->Data[offset]; @@ -298,8 +320,16 @@ void RoadRunnerIntracellular::set_parameter_value(std::string species_name, doub { rrc::RRVectorPtr vptr; + // see get_parameter_value(): operator[] would silently write column 0 for an unknown + // species, i.e. corrupt a different species than the caller named. + std::map::const_iterator it = species_result_column_index.find( species_name ); + if( it == species_result_column_index.end() ) + { + librr_unknown_species_fatal( species_name, __FUNCTION__ ); + } + vptr = rrc::getFloatingSpeciesConcentrations(this->rrHandle); - int idx = species_result_column_index[species_name]; + int idx = it->second; vptr->Data[idx] = value; // rrc::setFloatingSpeciesConcentrations(pCell->phenotype.molecular.model_rr, vptr); rrc::setFloatingSpeciesConcentrations(this->rrHandle, vptr); diff --git a/addons/libRoadrunner/src/librr_intracellular.h b/addons/libRoadrunner/src/librr_intracellular.h index 82b0b7f78..c4fb74092 100644 --- a/addons/libRoadrunner/src/librr_intracellular.h +++ b/addons/libRoadrunner/src/librr_intracellular.h @@ -47,9 +47,7 @@ class RoadRunnerIntracellular : public PhysiCell::Intracellular std::map phenotype_species; std::map species_result_column_index; - // rrc::RRHandle rrHandle = createRRInstance(); - rrc::RRHandle rrHandle; - // rrc::RRHandle rrHandle; + rrc::RRHandle rrHandle = nullptr; // created by start(), released by the destructor // rrc::RRVectorPtr vptr; rrc::RRCDataPtr result = 0; // start time, end time, and number of points @@ -58,9 +56,15 @@ class RoadRunnerIntracellular : public PhysiCell::Intracellular RoadRunnerIntracellular(); RoadRunnerIntracellular(pugi::xml_node& node); - + RoadRunnerIntracellular(RoadRunnerIntracellular* copy); - + + ~RoadRunnerIntracellular(); + + // owns rrHandle, so the implicit copies would double-free it; clone() is the way to copy + RoadRunnerIntracellular( const RoadRunnerIntracellular& ) = delete; + RoadRunnerIntracellular& operator=( const RoadRunnerIntracellular& ) = delete; + // rwh: review this Intracellular* clone() { @@ -70,6 +74,7 @@ class RoadRunnerIntracellular : public PhysiCell::Intracellular clone->substrate_species = this->substrate_species; clone->phenotype_species = this->phenotype_species; clone->custom_data_species = this->custom_data_species; + clone->start(); // must follow the assignments above: start() loads sbml_filename return static_cast(clone); } diff --git a/core/PhysiCell_cell.cpp b/core/PhysiCell_cell.cpp index 1f3e2646b..9a15e2759 100644 --- a/core/PhysiCell_cell.cpp +++ b/core/PhysiCell_cell.cpp @@ -654,7 +654,6 @@ Cell* Cell::divide( ) child->phenotype = phenotype; if (child->phenotype.intracellular){ - child->phenotype.intracellular->start(); child->phenotype.intracellular->inherit(this); } // #ifdef ADDON_PHYSIDFBA @@ -1124,8 +1123,6 @@ Cell* create_cell( Cell_Definition& cd ) pNew->functions = cd.functions; pNew->phenotype = cd.phenotype; - if (pNew->phenotype.intracellular) - pNew->phenotype.intracellular->start(); pNew->is_movable = cd.is_movable; // true; pNew->is_out_of_domain = false;