Skip to content

Conversation

@drbergman
Copy link
Collaborator

@drbergman drbergman commented Feb 2, 2025

provide a CSV file with similar structure to the substrate ICs csv header row: x,y,z,<substrate_01>,<substrate_02>,... where the list of substrates need not be all the substrates, only those with DCs being set. Each subsequent row is:

<x_coord>,<y_coord>,<z_coord>,<val_01>,<val_02>,...

where the coords must be specified and the vals can either be empty (<val_01>,,<val_03>,...) or a number

  • If empty, then nothing chagnes for the substrate in that column at that voxel
  • If a number, then the DC activation for that voxel-substrate pairing is set to the number

If the XML sets DCs, this will overwrite those values for which a number is supplied in the CSV. In other words, if a voxel on the boundary has a DC set to ON by the XML, parsing the CSV file can only change the value of the DC, not turn it OFF.

a sample project in sample_projects/dirichlet_from_file is provided. try it with make dirichlet-from-file-sample

provide a CSV file with similar structure to the substrate ICs csv
header row: x,y,z,<substrate_01>,<substrate_02>,...
where the list of substrates need not be all the substrates, only those with DCs being set
each subsequent row is:
<x_coord>,<y_coord>,<z_coord>,<val_01>,<val_02>,...
where the coords must be specified and the vals can either be empty (<val_01>,,<val_03>,...) or a number
If empty, then nothing chagnes for the substrate in that column at that voxel
If a number, then the DC activation for that voxel-substrate pairing is set to the number
@drbergman drbergman force-pushed the feature-dc-init-file branch from 0cec57e to c7860b4 Compare February 2, 2025 11:31
@drbergman drbergman requested a review from Copilot April 4, 2025 15:07
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 9 out of 16 changed files in this pull request and generated no comments.

Files not reviewed (7)
  • Makefile: Language not supported
  • sample_projects/Makefile-default: Language not supported
  • sample_projects/dirichlet_from_file/Makefile: Language not supported
  • sample_projects/dirichlet_from_file/config/PhysiCell_settings.xml: Language not supported
  • sample_projects/dirichlet_from_file/config/cell_rules.csv: Language not supported
  • sample_projects/dirichlet_from_file/config/cells.csv: Language not supported
  • sample_projects/dirichlet_from_file/config/dcs.csv: Language not supported
Comments suppressed due to low confidence (1)

BioFVM/BioFVM_microenvironment.cpp:1735

  • The function update_dirichlet_node is invoked here but it is not declared or defined in the provided diffs. Consider replacing this call with a defined function such as set_substrate_dirichlet_activation (or implementing update_dirichlet_node) to ensure the dirichlet condition is updated correctly.
microenvironment.update_dirichlet_node(voxel_ind, substrate_indices[ci], data[ci + 3]);

return;
}

void load_initial_conditions_from_matlab(std::string filename)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is unchanged except that the error at the bottom now goes to std::cerr rather than std::cout. It is green here only because the set_microenvironment_initial_condition function is refactored and that moved things around too much for git to track well.

return;
}

void load_initial_conditions_from_csv(std::string filename)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is unchanged except the comment // Do not include a header row. has been removed since you can (and probably should) included a header row. See comment on load_initial_conditions_from_matlab.

Note: you should include a header row to explicitly declare the substrates you are setting, rather than relying on their index in the substrate list.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In reviewing this PR, I found an error when loading DCs from a CSV without a header row. That mistake was traced back to the code for reading in substrates from a CSV (written by yours truly). So, that has been fixed here.

return;
}

void get_row_from_substrate_initial_condition_csv(std::vector<int> &voxel_set, const std::string line, const std::vector<int> substrate_indices, const bool header_provided)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is entirely unchanged. See comment on load_initial_conditions_from_matlab.

set_dirichlet_boundaries_from_file();
}

void set_dirichlet_boundaries_from_XML( void )
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new function is just the old end to set_microenvironment_initial_condition, refactored into its own function for simplicity and clarity of logic. This is especially useful as its caller (set_dirichlet_initial_condition) handles the loading of DC conditions from both XML and file.

}

void load_initial_conditions_from_matlab(std::string filename)
void set_dirichlet_boundaries_from_file( void )
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment on load_initial_conditions_from_matlab. This is a new function, not a rework of load_initial_conditions_from_matlab. It just shares enough code that git thinks it is reworking this.

@drbergman drbergman self-assigned this May 22, 2025
@drbergman drbergman requested a review from Copilot May 22, 2025 13:07
@drbergman drbergman assigned drbergman and unassigned drbergman May 22, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces functionality for initializing Dirichlet conditions from a CSV file. Key changes include updates to the main simulation and configuration files, new parsing routines for CSV‐based Dirichlet conditions in the BioFVM module, and corresponding build system modifications.

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
sample_projects/dirichlet_from_file/main.cpp Updates to main simulation code to invoke DC initialization
sample_projects/dirichlet_from_file/custom_modules/.h,.cpp Custom module updates with minimal changes
config/*.csv, config/PhysiCell_settings.xml New configuration files (CSV and XML) supporting DC initialization
Makefile, Makefile-default Build files updated to support the new project configuration
modules/PhysiCell_settings.cpp Added XML parsing logic for Dirichlet condition file settings
BioFVM/BioFVM_vector.{h,cpp} New function (dirichlet_csv_to_vector) to parse CSV rows for DC data
BioFVM/BioFVM_microenvironment.{h,cpp} Updates and new functions for setting and loading Dirichlet conditions

while( i < strlen( buffer ) )
{
if(buffer[i] == ',')
{
Copy link

Copilot AI May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inside 'dirichlet_csv_to_vector', the index 'ind' is incremented without checking whether it has reached the allocated size of the 'is_missing' and 'data' vectors, which could lead to an out-of-bounds access if the CSV row contains more fields than expected. Consider adding a bounds check before writing to these vectors.

Suggested change
{
{
if (ind >= is_missing.size())
{
std::cerr << "Error: Too many fields in the CSV row. Expected at most " << is_missing.size() << " fields." << std::endl;
std::cerr << "\tRow: " << buffer << std::endl;
exit(-1);
}

Copilot uses AI. Check for mistakes.
Comment on lines 1725 to 1726
<< "\tExpected: " << microenvironment.number_of_voxels() << std::endl
<< "\tFound: " << data.size() - 3 << std::endl
Copy link

Copilot AI May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning message incorrectly prints the expected value using 'microenvironment.number_of_voxels()' even though the check compares against (number_of_densities() + 3). This could be confusing; revise the error message to correctly reference the expected number of density data columns.

Suggested change
<< "\tExpected: " << microenvironment.number_of_voxels() << std::endl
<< "\tFound: " << data.size() - 3 << std::endl
<< "\tExpected: " << (microenvironment.number_of_densities() + 3) << " columns (3 for position + " << microenvironment.number_of_densities() << " for substrates)" << std::endl
<< "\tFound: " << data.size() << " columns" << std::endl

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant