Restore and modernize the complete AIM documentation - #5
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThe PR refreshes the Sphinx site with a new theme, styling, card-based navigation, module reference pages, input/output guidance, video tutorials, and updated README and CI documentation links. ChangesDocumentation site refresh
Estimated code review effort: 3 (Moderate) | ~25 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6938f0a0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| **HeatFit** also reports coefficient of determination, :math:`r^{2}`, value defined as: | ||
|
|
||
| .. math:: | ||
| r^{2} = 1 - \frac{SSE}{\sum (q_{i,exp} - \overline{q_{i,exp}})} |
There was a problem hiding this comment.
When users rely on the restored HeatFit reference to reproduce the reported goodness-of-fit, this denominator is documented as a raw sum of deviations, which should sum to approximately zero rather than total variance; the implementation and the corrected IsoFit page use squared deviations, so this formula can make the documented r² undefined or wildly wrong.
Useful? React with 👍 / 👎.
|
|
||
| .. math:: | ||
| RMSE = \sqrt{\frac{SSE}{N-M}},\\ | ||
| r^{2} = 1 - \frac{SSE}{\sum (P_{i,exp} - \overline{P_{i,exp}})} |
There was a problem hiding this comment.
For Virial fitting, the SSE just above is defined on ln(P), and src/Virial_fitting.m computes the variance over the stored log-pressure column; this line instead documents raw pressure deviations and also omits the square, so users trying to validate HeatFit's Virial output will use a different and invalid r² definition.
Useful? React with 👍 / 👎.
| * Thermal equilibrium exists between gas and solid phase. | ||
| * The heat transfer coefficient governing the heat transfer between column wall and bed is constant. | ||
| * The mass transfer resistance between solid and gas phase is governed by Linear Driving Force (LDF) model. | ||
| * The concentration, pressure, and temperature gradients in the axial directions are negligible. |
There was a problem hiding this comment.
In the BreakLab model assumptions, this says axial concentration, pressure, and temperature gradients are negligible, but the same restored section immediately defines z-dependent PDEs, axial dispersion, pressure-drop, and inlet/outlet boundary conditions; for the one-dimensional bed model the neglected gradients are radial, so the current wording contradicts the governing model and can mislead users about which transport terms are solved.
Useful? React with 👍 / 👎.
| - :math:`q = \frac{q_{sat} b P^{n}}{1 + b P^{n}}` | ||
| - :math:`q_{sat},\ b,\ n` | ||
| * - Dual-site Langmuir-Freundlich | ||
| - :math:`q = \frac{q_{sat, 1} b P^{n_{1}}}{1 + b_{1} P^{n_{1}}} + \frac{q_{sat,2} b_{2} P^{n_{2}}}{1 + b_{2} P^{n_{2}}}` |
There was a problem hiding this comment.
For users copying the documented dual-site Langmuir-Freundlich equation, the first site's numerator uses an unindexed b even though the denominator and parameter list define b_1; the implementation uses the first site's b1 parameter, so the docs currently publish the wrong expression here and in the matching HeatFit table.
Useful? React with 👍 / 👎.
1 participant
Summary
This corrects the overly minimal documentation introduced in #4. The redesign now preserves and promotes the repository's complete legacy technical documentation instead of leaving it hidden in generated HTML.
.blisocontractPreserved content inventory
.blisofilesValidation
git diff --checkpython ci/check_repository.pypython -m sphinx -W --keep-going -b html docs/source /tmp/aim-docs-modern-htmlDesign
The information architecture follows the core-mof-tools modernization approach: modern typography and cards at the entry point, then deep technical references underneath. Scientific detail is retained rather than replaced with marketing summaries.
Summary by CodeRabbit