-
-
Notifications
You must be signed in to change notification settings - Fork 109
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
Add muscle tutorial #471
base: develop
Are you sure you want to change the base?
Add muscle tutorial #471
Conversation
My suggestion here regarding the distribution release: Let's aim to merge this before the distribution, as it would help citing and starting a userbase/contributing community for OpenDiHu. We anyway plan to maintain this. For the code itself, let's not rush it. Since this is the first tutorial of this kind, I would like to do a sanity check as well. |
autopep8 --in-place --exit-code --aggressive --ignore=E402 --max-line-length=120 file.py
pip3 install --user autopep8 autopep8 --recursive --in-place --aggressive --exit-code --ignore E402 --max-line-length 120 .
- Exit on errors - Complain for undefined variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went ahead and fixed most of the linting issues and some other common requirements for tutorials. In terms of general structure, it looks good.
I have not tried to run it. I will once you tell me it is ready.
I have not reviewed the code of the solvers, as I lack the expertise. I trust that they have been validated elsewhere.
See a few more suggestions and questions to continue.
muscle-tendon-complex/.gitignore
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #477 and restrict this to the difference (let's better wait until it is merged).
Are all of these files generated with the default instructions, or are some rules only as "just in case"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I don't understand. Which default instructions are you referring to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant "are all these rules to ignore files that will actually be generated when I try to run the tutorial?".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes! Some when building and some when running
muscle-tendon-complex/README.md
Outdated
|
||
The muscle and tendon meshes are obtained from patient imaging. The interfaces of the tendons and the muscle do not perfectly match, which is a quite common issue due to the limitations of imaging methods and postprocessing tools. Nonetheless, preCICE coupling methods are robust and can handle meshes that do not perfectly match. | ||
|
||
TODO: Explain how is the muscle activated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this already covered later on by the following?
The electrical signal triggers chemical reactions which lead to the contraction of sarcomeres, the smallest contraction unit in the muscle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to make it clear in a new commit.
end_time = 1000.0 # [ms] end time of the simulation | ||
# [ms^-1] sampling frequency of stimuli in firing_times_file, in stimulations per ms, number before 1e-3 factor is in Hertz. | ||
stimulation_frequency = 100 * 1e-3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long comments have been moved to new lines, as a result of the aggressive option (I think) of the autopep8 and the column limit (120). Consider shortening or splitting some of these comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
muscle-tendon-complex/README.md
Outdated
|
||
- Download input files for OpenDiHu | ||
|
||
OpenDiHu requires input files hosted in [Zenodo](https://zenodo.org/records/4705982) which include CellML files (containing model equations) and mesh files. Downloading these files is necessary to simulate muscles and/or tendons with OpenDiHu. You can [directly download the necessary files](https://zenodo.org/record/4705982/files/input.tgz?download=1). Extract the files and place them in the `opendihu/examples/electrophysiology/` directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do they really need to be downloaded into the opendihu
source directory? What if that is a system directory? What if it is generally a frozen installation?
Can we move these to a local path, or to an OPENDIHU_EXAMPLES
directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand your point.
Would path/to/opendihu/examples/electrophysiology/
be better?
Or OPENDIHU_HOME/examples/electrophysiology/
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I mean that it is strange to first get OpenDiHu (imagine in the future a sudo apt install opendihu
) and then copy the examples into the installation directory (similarly to sudo apt install opendihu-examples
).
What I suggest is to decouple them from the actual installation (for the tutorial) and be able to use them from any directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmh I think you mean the input files and not the examples. The input files are downloaded separately because they are 1.5 GB and thus too big for GitHub.
Alternatives I see instead of moving the downloaded files are
- option 1: user has to define OPENDIHU_INPUT_DIR
# input files
# -----------
import os
input_dir = os.environ.get('OPENDIHU_INPUT_DIR')
fiber_file = input_dir + "/left_biceps_brachii_9x9fibers.bin"
- option 2: user has to edit variables.py
# input files
# -----------
input_dir = path/to/input
fiber_file = input_dir + "/left_biceps_brachii_9x9fibers.bin"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that option 2 is more intuitive for the user, while option 1 is better for scripting/automation.
Anything is fine, as long as this path if configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, I decided for option 2, because I think this is something I could generally do in OpenDiHu.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side-effect of having configuration files in Python script format (see opendihu/opendihu#44): How can I know if all variable names exist / are defined / are not misspelled / are needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point, at the moment there's no way to check that.
Co-authored-by: Gerasimos Chourdakis <[email protected]>
Co-authored-by: Gerasimos Chourdakis <[email protected]>
Co-authored-by: Gerasimos Chourdakis <[email protected]>
…nto add-muscle-tutorial
|
||
- Setup `$OPENDIHU_HOME` to your `.bashrc` file | ||
- Setup `$OPENDIHU_HOME` and `$OPENDIHU_INPUT_DIR` in your `.bashrc` file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a general terminology thing: When I see "input files", I understand configuration files for a specific simulation. How do these differ that the files included in this repository?
If they are specific to this tutorial, they could also be downloaded directly into this directory (and specified in the .gitignore
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What in opendihu we call input file are not what I understand as configuration files. They are files that specify meshes, i.e., geometry, and cellml models, i.e., equations. In the configuration files (the ones included in the tutorial, we give the parameters and user settings to use this files.
This would be a potential OpenDiHu tutorial. It has one muscle and three tendons (thus multi-coupling). We have the 4 participans running in OpenDiHu. Potentially, the tendon participants could also run in deal.ii.
Note that it contains no volume coupling.