Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve librr api #362

Open
wants to merge 9 commits into
base: development
Choose a base branch
from

Conversation

drbergman
Copy link
Collaborator

@drbergman drbergman commented Feb 13, 2025

As of opening this PR, this is ahead of the branch for PR #361 and is a bigger, better overhaul of the libRoadRunner API than that PR. Because it is bigger, I decided to make it a separate PR so interested parties could compare. Where #361 is refactoring/cleaning/fixing, this one would be a major change. It is backwards compatible except for minor modifications to config files setting intracellular mappings for libRoadRunner. See below.

Importantly, this update makes many of the mappings automatic with only modifying the config file, i.e., no custom coding necessary (users would still need to ensure the Makefile has the libRoadRunner macro, compile target, and other updates (see the sample_projects_intracellular/ode/ode_energy/Makefile for example). Look at the updated custom.cpp for the ode_energy project and observe how much simpler this is now!

Updated xml

A typical previous child element of the <intracellular> element would look like

<map PC_<key>="oxygen" sbml_species="Oxygen"></map >

where <key> could be any of substrate, phenotype, or custom_data.
Now, that element would be

<map type="io" physicell_name="intracellular oxygen" sbml_species="Oxygen"></map>

Some notes:

  • type is any of input, output, or io and defines whether the mapping is applied before or after updating the intracellular model AND whether PhysiCell values are being input into the model or output from the model (or both in the case of io)
  • physicell_name
    • matches the convention in the MaBoSS addon
    • can be any signal OR behavior dictionary entry
      • only a subset of signals are available to be assigned in an output mapping
      • those available for output:
        • intracellular <substrate_name> (assume sbml species is concentration and sets the total internalized using current cell volume)
        • volume (rescales all volumes)
        • damage
    • tokens from the previous version are also accepted, but only the volume ones (vtsc, vtsn, and vff) should be considered best practice
      • streamlines validation of the models to ensure no two mappings set the same value
      • the other tokens could be replaced by signals/behaviors.
    • among all output mappings (output or io), the physicell_name's must be unique
  • sbml_species
    • same convention as before
    • among all input mappings (input or io), the sbml_species's must be unique

- introduce `update_time_step` in `RoadRunnerIntracellular`
    - set for each instance of `RoadRunnerIntracellular`
    - this controls the timestep for the model
    - will be set to `intracellular_dt` if not specified in the xml for the given cell type
    - add the `intracellular_dt` element as a child of the `intracellular` element in the xml
    - could be set on a cell-by-cell basis if desired
- introduce `previous_update_time` in `RoadRunnerIntracellular`
    - tracks the previous update time
    - used to determine how long to run the ODE when updating
    - set to `current_time` when copying/cloning
- fix `next_librr_run` in `RoadRunnerIntracellular`
    - was not being updated and thus ODE models were running at every time step
    - check `current_time >= next_librr_run - 0.5 * diffusion_dt` to determine if an update is needed
- cleaned up librr files
- fixed and cleand ode-energy-sample
    - it was running the intracellular models in `main.cpp` and in the update cells function
    - it seemed to try to set intracellular_dt inside the intracellular element of the xml
    - messy custom.cpp and custom.h
a low-level implementation of the delay differential equation solver
before, only new ones were validated. those "copied" from previously read defs were not
Presumably, it was assumed that they fully inherited their parent's model, but as of now, they read the xml to get their model, so they need to be validated.
- `<map type="output" physicell_name="custom:intra_energy" sbml_species="Energy"></map>`
- `type` is any of `input`, `output`, `io` to indicate whether it is before or after (or both) the update
- `physicell_name` can be either a signal/behavior or one of the previous tokens
- `sbml_species` is the name of the species in the SBML model
- inputs are validated to ensure all sbml_species are unique
- outputs are validated to ensure all physicell_names are unique
- on `start()`, any signals/behaviors are validated to be in the relevant dictionary
- also output helpful info when using old, redundant tokens (non-volume ones)
multiple cells would be vying to all set the same voxel concentration

- also, clean up ode-energy-sample custom.cpp
@rheiland
Copy link
Collaborator

Still reviewing, but really liking this so far! Obviously, we'll need to synchronize changes for the Studio at release. I do wonder if we should expand the scope of this sample project a bit in order to demo/test for other options, e.g., <map type="input" ... .

Unless I'm missing something, I don't think we want dt_intracellular in the <overall>:
https://github.com/drbergman/PhysiCell/blob/improve-librr-api/sample_projects_intracellular/ode/ode_energy/config/PhysiCell_settings.xml#L94

@drbergman
Copy link
Collaborator Author

drbergman commented Feb 17, 2025

Great! And roger that on the studio updates. Hopefully pretty straightforward since we're now using signals/rules.

I think that's a good idea to at least have one <map type="input" element. However, I'm failing to think of a reasonable input for this model. We want it to stay biologically accurate...

On intracellular_dt, the element in the <overall> node sets the PhysiCell::intracellular_dt variable, which now that I'm looking at that more closely did nothing until this PR uses it as the default libRR update_time_step (for when no <intracellular_dt> node was found in <intracellular>). We could enforce explicit setting of this for each cell type rather than falling back on PhysiCell::intracellular_dt.

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.

2 participants