Expand Chinese documentation; add advanced examples & API refs#45
Expand Chinese documentation; add advanced examples & API refs#45chaoming0625 merged 6 commits intomainfrom
Conversation
…rect.Change the details of MixIons.
Reviewer's GuideThis PR overhauls the documentation by fully translating and expanding the Chinese tutorial notebooks for Channel, Ion, and Cell. It reorganizes each tutorial into clear “usage” and “custom modeling” sections, injects numerous code examples (including HTC neuron construction, detailed channel/ion definitions, dynamic simulations), and adds two advanced example notebooks (thalamic neuron and E-I network simulations). The advanced tutorials and API stubs are also renamed/localized, with new images and consistency updates across docs. Class diagram for SingleCompartment and MultiCompartment neuron modelsclassDiagram
class Cell {
<<abstract>>
}
class SingleCompartment {
+channels
+add_channel()
+simulate()
}
class MultiCompartment {
+morphology
+channels
+add_compartment()
+add_channel()
+simulate()
}
Cell <|-- SingleCompartment
Cell <|-- MultiCompartment
SingleCompartment o-- Channel
MultiCompartment o-- Channel
MultiCompartment o-- morphology : has
class Channel
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @chaoming0625 - I've reviewed your changes - here's some feedback:
- This PR is extremely large and covers multiple tutorials, APIs, and example notebooks—consider splitting it into smaller, focused PRs (e.g., one per tutorial or section) to simplify review and integration.
- There are many inline base64‐encoded images in the notebooks that bloat the diff; please move those images to external static files and reference them instead to keep diffs manageable.
- The notebooks include extensive execution outputs and warnings—please clear outputs (or use nbstripout) before committing to reduce noise in the diff.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- This PR is extremely large and covers multiple tutorials, APIs, and example notebooks—consider splitting it into smaller, focused PRs (e.g., one per tutorial or section) to simplify review and integration.
- There are many inline base64‐encoded images in the notebooks that bloat the diff; please move those images to external static files and reference them instead to keep diffs manageable.
- The notebooks include extensive execution outputs and warnings—please clear outputs (or use nbstripout) before committing to reduce noise in the diff.
## Individual Comments
### Comment 1
<location> `docs/tutorial/ion-zh.ipynb:327` </location>
<code_context>
+ "\n",
+ " def reset_state(self, V, batch_size=None):\n",
+ " ca_info = self.pack_info()\n",
+ " nodes = brainstate.graph.nodes(self, Channel, allowed_hierarchy=(1, 1)).values()\n",
+ " self.check_hierarchies(type(self), *tuple(nodes))\n",
+ " for node in nodes:\n",
+ " node.reset_state(V, ca_info, batch_size=batch_size)"
+ ],
</code_context>
<issue_to_address>
Type Channel is referenced but not defined in this code cell.
'Channel' is used but not defined or imported in this cell, which may cause a NameError. Please ensure it is available in the current context.
</issue_to_address>
### Comment 2
<location> `docs/tutorial/ion-zh.ipynb:455` </location>
<code_context>
+ " def derivative(self, C, V):\n",
+ " ICa = self.current(V, include_external=True)\n",
+ " drive = ICa / (2 * u.faraday_constant * self.d)\n",
+ " drive = u.math.maximum(drive, u.math.zeros_like(drive))\n",
+ " return drive + (self.C_rest - C) / self.tau"
+ ],
</code_context>
<issue_to_address>
The use of maximum() may mask negative calcium influx.
Setting the drive term to maximum(drive, 0) prevents modeling calcium efflux. Confirm if only inward flow is intended; otherwise, this may be too restrictive.
</issue_to_address>
### Comment 3
<location> `docs/advanced_tutorial/examples/sc05.ipynb:106` </location>
<code_context>
- " super().__init__(size, V_initializer=V_initializer, V_th=20. * u.mV, solver=solver)\n",
- "\n",
- " # 膜面积参数\n",
- " self.area = 1e-3 / (2.9e-4 * u.cm** 2)\n",
- "\n",
- " # 钠离子通道\n",
</code_context>
<issue_to_address>
Area calculation uses a hardcoded value that may reduce flexibility.
Parameterize the membrane area or add clear documentation explaining the chosen formula to improve code flexibility and maintainability.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
" def __init__(\n",
" self,\n",
" size,\n",
" gKL=0.01 * (u.mS / u.cm **2), # 钾漏通道电导\n",
" V_initializer=brainstate.init.Constant(-65. * u.mV), # 初始膜电位\n",
" solver: str = 'ind_exp_euler' # 积分方法\n",
" ):\n",
" super().__init__(size, V_initializer=V_initializer, V_th=20. * u.mV, solver=solver)\n",
"\n",
" # 膜面积参数\n",
" self.area = 1e-3 / (2.9e-4 * u.cm** 2)\n",
=======
" def __init__(\n",
" self,\n",
" size,\n",
" gKL=0.01 * (u.mS / u.cm **2), # 钾漏通道电导\n",
" V_initializer=brainstate.init.Constant(-65. * u.mV), # 初始膜电位\n",
" solver: str = 'ind_exp_euler', # 积分方法\n",
" membrane_area=None # 膜面积参数,可选,默认为1e-3/(2.9e-4 cm^2)\n",
" ):\n",
" super().__init__(size, V_initializer=V_initializer, V_th=20. * u.mV, solver=solver)\n",
"\n",
" # 膜面积参数\n",
" # 默认面积计算: 1e-3 / (2.9e-4 * u.cm**2) 来源于文献/实验数据,1e-3为细胞体积(单位L),2.9e-4为单位体积的面积(cm^2/L)\n",
" if membrane_area is None:\n",
" self.area = 1e-3 / (2.9e-4 * u.cm**2)\n",
" else:\n",
" self.area = membrane_area\n",
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `docs/advanced_tutorial/examples/sc03.ipynb:98` </location>
<code_context>
- " self.na = braincell.ion.SodiumFixed(in_size, E=50. * u.mV)\n",
- " self.na.add_elem(\n",
- " # 最大电导\n",
- " INa=braincell.channel.INa_TM1991(in_size, g_max=(100. * u.mS * u.cm **-2) * area, V_sh=-63. * u.mV)\n",
- " )\n",
- "\n",
</code_context>
<issue_to_address>
Multiplying g_max by area may lead to unit inconsistencies.
Since g_max is already in mS/cm^2, multiplying by area changes it to total conductance, which may not be appropriate. Please verify that INa_TM1991 expects conductance density and that this calculation does not cause a unit mismatch.
</issue_to_address>
### Comment 5
<location> `docs/advanced_tutorial/examples/sc03.ipynb:105` </location>
<code_context>
- " self.k = braincell.ion.PotassiumFixed(in_size, E=-90 * u.mV)\n",
- " self.k.add_elem(\n",
- " # 最大电导\n",
- " IK=braincell.channel.IK_TM1991(in_size, g_max=(30. * u.mS * u.cm** -2) * area, V_sh=-63. * u.mV)\n",
- " )\n",
- "\n",
</code_context>
<issue_to_address>
Potential unit mismatch in g_max for potassium channel as well.
Verify whether g_max should be a conductance density or total conductance for the potassium channel to avoid incorrect simulation behavior.
</issue_to_address>
### Comment 6
<location> `docs/advanced_tutorial/examples/sc02.ipynb:88` </location>
<code_context>
+ "cell_type": "code",
+ "source": [
+ "# 生成电压序列\n",
+ "vs = u.math.arange(-100 * u.mV, 0 * u.mV, 0.1 * u.mV)"
+ ],
+ "id": "43f7945b1fe9233f",
</code_context>
<issue_to_address>
Consider including the endpoint in the voltage range.
arange excludes the stop value, so 0 mV will not be part of the sequence. Use linspace or adjust the stop value if you want to include 0 mV.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
"# 生成电压序列\n",
"vs = u.math.arange(-100 * u.mV, 0 * u.mV, 0.1 * u.mV)"
=======
"# 生成电压序列\n",
"vs = u.math.arange(-100 * u.mV, 0.1 * u.mV, 0.1 * u.mV)"
>>>>>>> REPLACE
</suggested_fix>
### Comment 7
<location> `docs/apis/braincell.channel.rst:29` </location>
<code_context>
+ ICaGrc_Ma2020
+
+
+Hypterpolarization-Activated Channels
+-------------------------------------
+
</code_context>
<issue_to_address>
Typo: 'Hypterpolarization' should be 'Hyperpolarization'.
The section header contains a typo; please correct it to 'Hyperpolarization-Activated Channels'.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
Hypterpolarization-Activated Channels
-------------------------------------
=======
Hyperpolarization-Activated Channels
------------------------------------
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| " nodes = brainstate.graph.nodes(self, Channel, allowed_hierarchy=(1, 1)).values()\n", | ||
| " self.check_hierarchies(type(self), *tuple(nodes))\n", | ||
| " for node in nodes:\n", |
There was a problem hiding this comment.
issue (bug_risk): Type Channel is referenced but not defined in this code cell.
'Channel' is used but not defined or imported in this cell, which may cause a NameError. Please ensure it is available in the current context.
| " def derivative(self, C, V):\n", | ||
| " ICa = self.current(V, include_external=True)\n", | ||
| " drive = ICa / (2 * u.faraday_constant * self.d)\n", | ||
| " drive = u.math.maximum(drive, u.math.zeros_like(drive))\n", |
There was a problem hiding this comment.
question: The use of maximum() may mask negative calcium influx.
Setting the drive term to maximum(drive, 0) prevents modeling calcium efflux. Confirm if only inward flow is intended; otherwise, this may be too restrictive.
| " def __init__(\n", | ||
| " self,\n", | ||
| " size,\n", | ||
| " gKL=0.01 * (u.mS / u.cm **2), # 钾漏通道电导\n", | ||
| " V_initializer=brainstate.init.Constant(-65. * u.mV), # 初始膜电位\n", | ||
| " solver: str = 'ind_exp_euler' # 积分方法\n", | ||
| " ):\n", | ||
| " super().__init__(size, V_initializer=V_initializer, V_th=20. * u.mV, solver=solver)\n", | ||
| "\n", | ||
| " # 膜面积参数\n", | ||
| " self.area = 1e-3 / (2.9e-4 * u.cm** 2)\n", |
There was a problem hiding this comment.
suggestion: Area calculation uses a hardcoded value that may reduce flexibility.
Parameterize the membrane area or add clear documentation explaining the chosen formula to improve code flexibility and maintainability.
| " def __init__(\n", | |
| " self,\n", | |
| " size,\n", | |
| " gKL=0.01 * (u.mS / u.cm **2), # 钾漏通道电导\n", | |
| " V_initializer=brainstate.init.Constant(-65. * u.mV), # 初始膜电位\n", | |
| " solver: str = 'ind_exp_euler' # 积分方法\n", | |
| " ):\n", | |
| " super().__init__(size, V_initializer=V_initializer, V_th=20. * u.mV, solver=solver)\n", | |
| "\n", | |
| " # 膜面积参数\n", | |
| " self.area = 1e-3 / (2.9e-4 * u.cm** 2)\n", | |
| " def __init__(\n", | |
| " self,\n", | |
| " size,\n", | |
| " gKL=0.01 * (u.mS / u.cm **2), # 钾漏通道电导\n", | |
| " V_initializer=brainstate.init.Constant(-65. * u.mV), # 初始膜电位\n", | |
| " solver: str = 'ind_exp_euler', # 积分方法\n", | |
| " membrane_area=None # 膜面积参数,可选,默认为1e-3/(2.9e-4 cm^2)\n", | |
| " ):\n", | |
| " super().__init__(size, V_initializer=V_initializer, V_th=20. * u.mV, solver=solver)\n", | |
| "\n", | |
| " # 膜面积参数\n", | |
| " # 默认面积计算: 1e-3 / (2.9e-4 * u.cm**2) 来源于文献/实验数据,1e-3为细胞体积(单位L),2.9e-4为单位体积的面积(cm^2/L)\n", | |
| " if membrane_area is None:\n", | |
| " self.area = 1e-3 / (2.9e-4 * u.cm**2)\n", | |
| " else:\n", | |
| " self.area = membrane_area\n", |
| " self.na = braincell.ion.SodiumFixed(in_size, E=50. * u.mV)\n", | ||
| " self.na.add_elem(\n", | ||
| " # 最大电导\n", | ||
| " INa=braincell.channel.INa_TM1991(in_size, g_max=(100. * u.mS * u.cm **-2) * area, V_sh=-63. * u.mV)\n", |
There was a problem hiding this comment.
issue (bug_risk): Multiplying g_max by area may lead to unit inconsistencies.
Since g_max is already in mS/cm^2, multiplying by area changes it to total conductance, which may not be appropriate. Please verify that INa_TM1991 expects conductance density and that this calculation does not cause a unit mismatch.
| " self.k = braincell.ion.PotassiumFixed(in_size, E=-90 * u.mV)\n", | ||
| " self.k.add_elem(\n", | ||
| " # 最大电导\n", | ||
| " IK=braincell.channel.IK_TM1991(in_size, g_max=(30. * u.mS * u.cm** -2) * area, V_sh=-63. * u.mV)\n", |
There was a problem hiding this comment.
issue (bug_risk): Potential unit mismatch in g_max for potassium channel as well.
Verify whether g_max should be a conductance density or total conductance for the potassium channel to avoid incorrect simulation behavior.
| "# 生成电压序列\n", | ||
| "vs = u.math.arange(-100 * u.mV, 0 * u.mV, 0.1 * u.mV)" |
There was a problem hiding this comment.
suggestion: Consider including the endpoint in the voltage range.
arange excludes the stop value, so 0 mV will not be part of the sequence. Use linspace or adjust the stop value if you want to include 0 mV.
| "# 生成电压序列\n", | |
| "vs = u.math.arange(-100 * u.mV, 0 * u.mV, 0.1 * u.mV)" | |
| "# 生成电压序列\n", | |
| "vs = u.math.arange(-100 * u.mV, 0.1 * u.mV, 0.1 * u.mV)" |
| Hypterpolarization-Activated Channels | ||
| ------------------------------------- |
There was a problem hiding this comment.
issue (typo): Typo: 'Hypterpolarization' should be 'Hyperpolarization'.
The section header contains a typo; please correct it to 'Hyperpolarization-Activated Channels'.
| Hypterpolarization-Activated Channels | |
| ------------------------------------- | |
| Hyperpolarization-Activated Channels | |
| ------------------------------------ |
|
@sourcery-ai title |
Summary by Sourcery
Revise and enrich Chinese documentation across tutorials and examples, translating headings, expanding content for Channel, Ion, and Cell modules, and adding new advanced example notebooks and API reference stubs.
Documentation: