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

Fix backwards compatibility of CSTR #311

Merged

Conversation

AntoniaBerger
Copy link
Collaborator

Fixes #308

Before _constSolidVolume was always zero because the state of init_liquid_volume was declared after the parameters.

To fix this, there is now a variable init_liquid_volume to calculate _constSolidVolume even if the state is not set.
Additionally the calculation of _constSolidVolume has been fixed:

Before we calculated

$$ V^{s} = V^{l}*(1-\varepsilon) $$

which is wrong because we distinguish between the total volume of the reaction $V^{total}$ and the volumes of the liquid and solid phases.
Instead we now calculate

$$ V^{s} = V^{total}(1-\varepsilon) = \frac{V^{l}}{(1-\varepsilon)} - V^{l}. $$

For the last equation we used that $(1-\varepsilon) = \frac{V^{l}}{V^{l} + V^{s}} \Leftrightarrow \frac{V^{l}}{(1-\varepsilon)} - V^{l}$

@jbreue16 jbreue16 changed the title Fix for backwards compatibility of the parameter porosity and init_volume of the CSTR Fix backwards compatibility of CSTR Nov 12, 2024
@jbreue16
Copy link
Contributor

Looks good to me, thanks for catching and fixing this !

@jbreue16 jbreue16 added the bug label Nov 12, 2024
@jbreue16 jbreue16 merged commit d34b5d7 into master Nov 12, 2024
4 checks passed
@jbreue16 jbreue16 deleted the fix/No_concentration_decrease_during_binding_in_CSTR branch November 12, 2024 13:01
@github-actions github-actions bot locked and limited conversation to collaborators Nov 12, 2024
@jbreue16 jbreue16 added this to the v5.0.1 milestone Nov 13, 2024
@jbreue16
Copy link
Contributor

jbreue16 commented Dec 9, 2024

It should have been
$V_s = V_\text{total} (1-\varepsilon) = V_l \frac{1-\varepsilon}{\varepsilon}$.
Also, we should add a check for zero division.
The corrections are added to PR #339

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

No concentration decrease during binding in CSTR
2 participants