Skip to content

Conversation

@AntoniaBerger
Copy link
Collaborator

@AntoniaBerger AntoniaBerger commented Sep 28, 2025

This PR introduce a generalized reaction management system that allows multiple independent reaction types (liquid, solid, cross-phase) to coexist within a single unit operation and particle, enabling more flexible reaction modeling.

Key Changes

Core Infrastructure

  • New ReactionSystem class manages dynamic reaction models across different phases (cross_phase, liquid, pore, solid) within a unified structure
  • Phase-aware reaction organization with dedicated vectors for each phase type
  • Centralized configuration, discretization, and memory management

Separate cross phase reactions and single phase reactions

  • By default reactions are implemented as single phase reactions.
  • New single phase MASS_ACTION_LAW
  • Rename old MASS_ACTION_LAW to MASS_ACTION_LAW_CROSS_PHASE

Cell Kernel

  • Updated BindingCellKernel.hpp to handle multiple simultaneous reaction types
  • Integrated residual computation for:
    • Cross-phase reactions
    • Pure liquid-phase reactions
    • Pure solid-phase reactions
  • Consistent Jacobian generation for all reaction types

Interface Details

New Parameters Introduced

  • NREAC_LIQUID - Number of liquid phase reactions
  • NREAC_CROSS_PHASE - Number of cross-phase reactions
  • NREAC_SOLID - Number of solid phase reactions
  • Drop for single phase reactions (MASS_ACTION_LAW and MICHAELIS_MENTEN) the _bulk description

Testing

  • Fixed test suite validates:
    • Individual reaction types
    • Combined reaction scenarios
    • Cross-phase modifier effects
    • Jacobian accuracy (AD vs. analytic)
    • Two GRM tests fail in release mode but not in debug mode: this needs further investigation
    • Benchmark tests are covered in CADET-Varification (Add multiple reaction test CADET-Verification#57)

@AntoniaBerger AntoniaBerger self-assigned this Sep 28, 2025
@AntoniaBerger AntoniaBerger force-pushed the feature/add_multiple_reaction_generalized_unit branch from 8c518d1 to c1edc57 Compare September 29, 2025 14:03
@AntoniaBerger AntoniaBerger force-pushed the feature/add_multiple_reaction_generalized_unit branch 8 times, most recently from baf36e3 to 4627e37 Compare October 1, 2025 15:34
@AntoniaBerger AntoniaBerger force-pushed the feature/add_multiple_reaction_generalized_unit branch 3 times, most recently from ffa9f66 to 67095d8 Compare November 19, 2025 15:39
@AntoniaBerger AntoniaBerger force-pushed the feature/add_multiple_reaction_generalized_unit branch 2 times, most recently from 863d35b to f352912 Compare December 1, 2025 09:42
@jbreue16
Copy link
Contributor

jbreue16 commented Dec 1, 2025

this is the continuation of #449 and #453, right? I think the PR title can be updated

Copy link
Contributor

@jbreue16 jbreue16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so cool to have, very valuable work !

Seems like 10 crystallization tests are failing and I don't think this is solely due to CI shenanigans, since in that case it's always been just one or two that fail. Also, this PR changes reactions and crystallization is implemented as an reaction and specific care should be taken here with these tests.
@AntoniaBerger do you have an idea on what could be going wrong here? have you tested if its really just the CI thats failing us? You could e.g. run some of the tests defined in cadet-verification to double check if the changes actually broke crystallization

@AntoniaBerger
Copy link
Collaborator Author

this is the continuation of #449 and #453, right? I think the PR title can be updated

Yes, at some point I found my self rewriting it for every unit operation. I should change the title of the PR.

@AntoniaBerger AntoniaBerger changed the title Add multiple reaction generalized unit Add multiple reaction Dec 1, 2025
@AntoniaBerger
Copy link
Collaborator Author

have you tested if its really just the CI thats failing us? You could e.g. run some of the tests defined in cadet-verification to double check if the changes actually broke crystallization.

This is a great point! I will double check!

@AntoniaBerger AntoniaBerger force-pushed the feature/add_multiple_reaction_generalized_unit branch 2 times, most recently from 5a845a1 to 3a53b46 Compare December 2, 2025 16:01
@AntoniaBerger AntoniaBerger force-pushed the feature/add_multiple_reaction_generalized_unit branch from 5a23f0f to 3882248 Compare December 3, 2025 13:46
@AntoniaBerger
Copy link
Collaborator Author

All failing Crystallization tests are running in debug and release mode on my local computer. @jbreue16 can you reproduce this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants