expose and parse the mechanics grid size in XML#413
expose and parse the mechanics grid size in XML#413rheiland wants to merge 13 commits intoMathCancer:developmentfrom
Conversation
drbergman
left a comment
drbergman
left a comment
There was a problem hiding this comment.
Love that this exposes this part of the model! Looks like a straightforward implementation of it, which will make it easy for users to pick up. I like the decision to have the stdout print if it's not in the XML as a way to inform users about this change. I also like that this message will only be read by those who look carefully through the initial model logs, since it is something that can be dangerous (see the comment Randy opened this PR with).
I raised two points here for discussion / to address. First, this PR has mechanics_voxel_size defined both in PhysiCell_constants and in PhysiCell_settings. My vote is for having it within PhysiCell_settings, and that is where the XML parser puts it now.
Second, I think we should completely remove mechanics_voxel_size from the main.cpp files. We should use the new PhysiCell_settings.mechanics_voxel_size within the called create_cell_container_for_microenvironment.
Looking forward to discussing and eventually merging! I heard that changes to Studio are already in dev (not sure branch or not), but happy to help any way I can there.
|
|
||
| // set mechanics voxel size, and match the data structure to BioFVM | ||
| double mechanics_voxel_size = 30; | ||
| double mechanics_voxel_size = PhysiCell_settings.mechanics_voxel_size; |
There was a problem hiding this comment.
In the main.cpp, mechanics_voxel_size is exclusively used to pass in to create_cell_container_for_microenvironment. Should we continue defining this variable scoped only to the main function for this? Or just rely on the PhysiCell constant we are defining?
core/PhysiCell_constants.cpp
Outdated
| double phenotype_dt = 6.0; | ||
| double intracellular_dt = 0.01; | ||
|
|
||
| double mechanics_voxel_size = 30.0; |
There was a problem hiding this comment.
As best I can tell, this mechanics_voxel_size is not being used anywhere else. Should we remove it from PhysiCell_constants and have it only exist within PhysiCell_settings?
There was a problem hiding this comment.
I had mistakely convinced myself that it need to be in both _constants and _settings, but I was apparently wrong. I removed it from _constants. Also, I agree that it's cleaner to just pass PhysiCell_settings.mechanics_voxel_size in all main.cpp create_cell_container_for_microenvironment and I pushed those changes.
2 participants
The mechanics voxel size has had a default of 30 and has been hard-coded in main.cpp since PhysiCell came out. This PR makes it possible to easily modify that value in the XML config file. A new tag
<mechanics_voxel_size>now exists in the<options>block. AndPhysiCell_settings.mechanics_voxel_sizenow exists and =30.For all sample projects (and the ode intracellular project):
<mechanics_voxel_size>30</mechanics_voxel_size>into the config filesmain.cppto no longer hard-code 30; rather, read it fromPhysiCell_settings.mechanics_voxel_sizeAdded
unit_tests/mech_voxel_size/which contains a directory structure and files suitable for/user_projects:/configfiles for different sized mechanics voxel sizes:test_mech_grid_size_{20,25,30}.xmlmain.cppof this unit test has commented outmicroenvironment.simulate_diffusion_decay, primarily to see if it would affect the following issue (it didn't).We may want to discuss some (perhaps) surprising results from just changing the mechanics voxel size, as the above unit test should demonstrate: we end up with a different number of agents. Aside from this, even if the number of agents had been the same, we may end up with different cell scalars, e.g., pressure, if those scalars depend on the neighbor cells which depends on the Moore neighborhood which depends on the mechanics voxel size.