Skip to content

Conversation

@jbreue16
Copy link
Contributor

@jbreue16 jbreue16 commented Aug 19, 2025

double check and document interface changes

  • remove enable_dg flag, eigen will become a required dependency
  • implement generalized interface for old FV and LRM_DG units. also change this in createLWE, breakthrough.py, etc
  • implement generalized modelbuilder, only new interface should be exposed
  • update names of input fields
    • rename: PAR_BOUNDARY_ORDER -> FV_BOUNDARY_ORDER, remove par suffix from particle cell/element spacing types, e.g. EQUIDISTANT_PAR
    • remove field PARTICLE_TYPE. instead, we will have explicit bools for what effects are included in the particle model, i.e. has_film_diffusion has_pore_diffusion, has_surface_diffusion, which default to false. add automatic check for impossible/unsupported combinations. if a field is set to false, the corresponding parameter, if provided, is ignored.
    • rename PAR_DIFFUSION to PORE_DIFFUSION and PAR_SURFDIFFUSION to SURFACE_DIFFUSION
  • update field UNIT_TYPE :
    • if GRM, LRMP are specified, the respective particles are used automatically but check if the fields are inconsistent if they exist.
  • add documentation, see below for interface changes
  • [x] make modified newton the new default, see Change default for USE_MODIFIED_NEWTON to true #478

@schmoelder
Copy link
Contributor

remove field UNIT_TYPE from standard configuration, but keep it for backwards compatibility. if GRM, LRMP are specified, the respective particle should be used automatically. Instead, for the standard configuration we need a new field indicating the dimensionality, and maybe the direction of the flow or, even better, the device geometry/type. what to do with FV units?

I would not recommend removing the UNIT_TYPE field. We might want to add different unit operations (e.g. filtration) which are not part of the generalized column family.

@jbreue16 jbreue16 changed the base branch from master to jb/dev August 19, 2025 13:57
@jbreue16
Copy link
Contributor Author

jbreue16 commented Aug 19, 2025

I would not recommend removing the UNIT_TYPE field. We might want to add different unit operations (e.g. filtration) which are not part of the generalized column family.

It'll stay, the comment was not updated. Old specifications such as GRM, LRMP will imply the particle type and $N^p=1$, which is not just backwards compatibility but a feature of the interface. The new input for a generic column will be something like AXIAL_FLOW_COLUMN, RADIAL_FLOW_COLUMN, indicating the geometry. Another, separate field will be used for the dimensionality and default to 1D

@jbreue16 jbreue16 changed the base branch from jb/dev to feature/dg2d August 19, 2025 15:44
@jbreue16 jbreue16 force-pushed the jb/generalized_interface branch 7 times, most recently from 748a5ca to 83eadd8 Compare August 20, 2025 14:47
@jbreue16 jbreue16 self-assigned this Aug 21, 2025
@jbreue16 jbreue16 force-pushed the jb/generalized_interface branch 2 times, most recently from fb813bf to bf148e8 Compare August 21, 2025 16:44
@jbreue16
Copy link
Contributor Author

jbreue16 commented Aug 21, 2025

Interface changes:

  • UNIT_TYPE can now be one of the legacy names or COLUMN_MODEL_1D, _2D, RADIAL_COLUMN_MODEL_1D and so on; ie different flow directions/geometries are added as prefix, dimensionality as sufffix
  • field NPARTYPE defaults to zero, i.e. becomes mandatory when particles are used, except when old unit_types are specified, then default is 1
  • new group particle_type_000 for every particle type with the respective index
  • NBOUND is moved into the particle group(s)
  • multiplexes (e.g. FILM_DIFFUSION_MULTIPLEX) exclude partype multiplex. some multiplexes which only had partype, e.g. ADSORPTION_MODEL_MULTIPLEX, REACTION_MODEL_PARTICLES_MULTIPLEX are deprecated.
  • New fields are used to specify parameter dependence as used in parameter sensitivities:, FILM_DIFFUSION_PARTYPE_DEPENDENT, BINDING_PARTYPE_DEPENDENT and REACTION_PARTYPE_DEPENDENT, PAR_SURFDIFFUSIN_DEP_PARTYPE_DEPENDENT, PAR_RADIUS_PARTYPE_DEPENDENT, PAR_CORERADIUS_PARTYPE_DEPENDENT, PAR_POROSITY_PARTYPE_DEPENDENT, PORE_ACCESSIBILITY_PARTYPE_DEPENDENT, PAR_DIFFUSION_PARTYPE_DEPENDENT,
    the internal MultiplexMode remains the same and is derived as before but with the additional information from the new partype dependence field.
  • parameter dependencies for sensitivities now must be specified via PAR_RADIUS_PARTYPE_DEPENDENT etc. otherwise, parameter is assumed to be dependent on the particle type.
  • REACTION_MODEL_PARTICLES -> REACTIN_MODEL in each particle group
  • if needed, particles have their own discretization group (only General rate particle for now)
  • initial conditions of $c^p, c^s$ were moved to particles: INIT_CS (formerly INIT_Q) and INIT_CP. in the case of particle type independent bindings, it is checked if initial conditions are the same across particle models
  • NPAR -> NCELLS
  • NCOL-> NCELLS
  • SPATIAL_METHOD is mandatory for both bulk and GRM particle
  • multiplexing: only 0 and 1: 0 -> component dependent, 1 -> section and component dependent. Sensitivity dependence is handled via new field {param_name}_PARTYPE_DEPENDENT
  • introduced fields HAS_FILM_DIFFUSION HAS_PORE_DIFFUSION HAS_SURFACE_DIFFUSION
  • renamed PAR_DIFFUSION to PORE_DIFFUSION and PAR_SURFDIFFUSION to SURFACE_DIFFUSION
  • renamed COL_DISPERSION_AXIAL in 2D units
  • document choice of linear solvers for generalized units

@jbreue16 jbreue16 force-pushed the jb/generalized_interface branch 8 times, most recently from 68800e8 to b7f6ba4 Compare August 25, 2025 11:37
@jbreue16 jbreue16 force-pushed the jb/generalized_interface branch 2 times, most recently from 053ae52 to f01feea Compare August 26, 2025 10:16
jbreue16 added a commit that referenced this pull request Aug 26, 2025
This commit additionally reverts changes made to the bulk reaction
interface made in 900bb46, since they
are planned to be reworked.
@jbreue16 jbreue16 force-pushed the jb/generalized_interface branch 5 times, most recently from 9267610 to dc516df Compare August 27, 2025 13:46
This commit unifies the column model builder so that a unified unit
operation interface is exposed while internally unit operations can have
both different or unified implementations.
Makes input field `SPATIAL_METHOD` mandatory.
@jbreue16 jbreue16 force-pushed the jb/generalized_interface branch 4 times, most recently from 08e5157 to 0befc7e Compare August 28, 2025 16:05
@jbreue16
Copy link
Contributor Author

Seems like the CI is triggered for commits changing files listed under paths-ignore on PRs that have changes in other places too

@jbreue16 jbreue16 added this to the v6.0.0 milestone Aug 28, 2025
@jbreue16 jbreue16 force-pushed the jb/generalized_interface branch from c278d4e to 9a221b0 Compare August 28, 2025 21:04
* all GRM LRMP LRM 1D and 2D units

* rename fields to `PORE_DIFFUSION` and `SURFACE_DIFFUSION`

* 2D units: rename `COL_DISPERSION` to `COL_DISPERSION_AXIAL`

* replace `PARTICLE_TYPE` by transport model bools
@jbreue16 jbreue16 force-pushed the jb/generalized_interface branch from 2bf8e90 to d9b1305 Compare August 28, 2025 22:02
@jbreue16 jbreue16 merged commit 4cc363a into master Aug 28, 2025
4 checks passed
jbreue16 added a commit that referenced this pull request Aug 28, 2025
* all GRM LRMP LRM 1D and 2D units

* rename fields to `PORE_DIFFUSION` and `SURFACE_DIFFUSION`

* 2D units: rename `COL_DISPERSION` to `COL_DISPERSION_AXIAL`

* replace `PARTICLE_TYPE` by transport model bools
jbreue16 added a commit that referenced this pull request Aug 28, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in CADET Repositories Aug 28, 2025
@jbreue16 jbreue16 deleted the jb/generalized_interface branch August 28, 2025 22:15
@github-actions github-actions bot locked and limited conversation to collaborators Aug 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants