-
Notifications
You must be signed in to change notification settings - Fork 26
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
Radial flow for chromatography column models #120
Conversation
Hey Sam, @jbreue16, could you do us a favor and compile the branch for Windows? |
Hey Sam, Apart from this, the PR compiles and runs fine on my windows machine :) |
@schmoelder compiled for windows |
65d5f91
to
c18b15f
Compare
Do we still want this in the main branch or shall we better wait for the DG version? |
todo list moved to the upmost comment |
c18b15f
to
3914f3e
Compare
In CADET, different parameter-parameter dependencies can be specified and the user is free to use any of the implemented dependencies for dependent parameters. where Note, in case of axial dispersion, the dependency is implemented the following way: To specify this, set
Moreover, a flag can be set which only uses the absolute value of the independent parameter.
|
as mentioned in #121, we need to add |
f2e4c68
to
c2c8ce4
Compare
@jbreue16 and I had another idea regarding the implementation of the dependencies. Right now, e.g. in case of axial dispersion we have the following situation:
Here, a "base" value for D_ax is set which is then multiplied (this is hard-coded) with some configurable dependency on the velocity (which is also hard coded).
However, parameter dependencies might not always be multiplicative but could also be additive. For this purpose we also had to implement a Wouldn't it make sense to include the respective parameter in the arguments of Maybe something like this:
At first glance, Jan and I couldn't see any downsides but maybe you have another thought, @sleweke? @sleweke UPDATE (26.05.2023): UPDATE (16.06.2023): |
4a48aac
to
68801ae
Compare
2c63233
to
5c5c55c
Compare
5c5c55c
to
f8f20c0
Compare
a6a6727
to
ebd87e2
Compare
Co-authored-by: jbreue16 <[email protected]>
ebd87e2
to
59b5434
Compare
Spatial FV discretization units only Co-authored-by: jbreue16 <[email protected]>
Adds infrastructure code (e.g., factories, interfaces) to allow parameters to depend on other parameters. Co-authored-by: jbreue16 <[email protected]>
Adds a parameter dependence that simply outputs the given parameter value.
Add radial flow and parameter dependence support to createLWE Axial dispersion for all FV units, film diffusion parameter dependence for LRMP FV unit Add tests for radial flow convection dispersion operators Enable radial dispersion coeff dependency in operators Parameter dependencies are work in progress and the interfaces might change up until the next release Co-authored-by: jbreue16 <[email protected]>
59b5434
to
1ec9150
Compare
Adds radial flow to chromatography column models. Only supports the upwind scheme as of now (i.e., no WENO) and uses constant dispersion coefficient.
The column model classes are now templates that take the type of the convection-dispersion-operator class. This required condensing the compilation down to one translation unit (TU) per column model.
For each column model, we add a second one with radial flow. That is, if
UNIT_TYPE
isGENERAL_RATE_MODEL
, then usingRADIAL_GENERAL_RATE_MODEL
will give you the one with radial flow. For the geometry, the fieldsCOL_RADIUS_INNER
andCOL_RADIUS_OUTER
have been added. A positive velocity indicates flow from inner to outer radius.To emphasize the difference of having a velocity field rather than a global velocity, we name the input field
VELOCITY_COEFF
, as opposed toVELOCITY
which is used for the axial flow modelsTo do