Feng (Project 1) #155
Feng (Project 1) #155
Conversation
adowling2
left a comment
adowling2
left a comment
There was a problem hiding this comment.
Please see feedback below.
| "id": "zEE6J3UhC1vu" | ||
| }, | ||
| "source": [ | ||
| "### 1A Calculating Alcohol Ingestion\n", |
There was a problem hiding this comment.
Missing "." to be consistent with other subsection titles.
| "id": "rFj5qSrOM1H9" | ||
| }, | ||
| "source": [ | ||
| "**Discussion:**" |
There was a problem hiding this comment.
Please provide answers to all discussion questions.
| "source": [ | ||
| "This function solves the ODE for dCa/dt, dCb/dt, and dBAC/dt that you made in part 1B.\n", | ||
| "\n", | ||
| "**Its important to note that this ODE assumes that a person of \"m\" weight drinks \"n\" number of drinks on a moderatley full stomach at a given rate then stops.**" |
There was a problem hiding this comment.
Please rephrase. I think you are saying that
| { | ||
| "cell_type": "markdown", | ||
| "source": [ | ||
| "## 1D. Example of Forward Euler Method for ODEs\n", |
There was a problem hiding this comment.
This contribution is confusing. Is the main goal to practice forward Euler? Or is the main goal to compare the full kinetic and simplified kinetic models? Please make this more clear. If the later, remove the forward Euler and restructure to emphasize comparing different models. Use the same integration scheme as earlier in the notebook.
| }, | ||
| "outputs": [], | ||
| "source": [ | ||
| "def SolveODE(CT, tmax, f):\n", |
There was a problem hiding this comment.
Change function name to solve_ode. Best practice in Python is to not capitalize function names (that often means classes).
| }, | ||
| "outputs": [], | ||
| "source": [ | ||
| "def Model_info(n, m, tmax):\n", |
There was a problem hiding this comment.
Choose a more descriptive function name. Model_info is ambiguous.
There was a problem hiding this comment.
Likewise, the doc string does not explain what the function does.
| "# Add your solution here\n", | ||
| "### BEGIN SOLUTION\n", | ||
| "# Plot CA and BAC\n", | ||
| "fig, ax1 = plt.subplots(figsize=(6.4, 4), dpi=100)\n", |
There was a problem hiding this comment.
The font sizes in the axes labels and axes numbers are too small. Please see the instructions for publication quality plots.
| "id": "VK-3NOub6R64" | ||
| }, | ||
| "source": [ | ||
| "**Discussion:**" |
There was a problem hiding this comment.
Please provide answers to all discussion questions.
| { | ||
| "cell_type": "code", | ||
| "source": [ | ||
| "def forward_euler(k, C0, tmax, dt):\n", |
There was a problem hiding this comment.
This function combines the logic for Euler's method with the specifics of the model. This is NOT best practice. Instead, we want to separate the logic for the general numerical method from the model details. Please see examples on the class website. Alternately, please remove Euler's method to instead focus on comparing the models.
2 participants
@adowling2