Crystallization reaction module

[sleweke]

Adds a reaction module for crystallization. The population balance is discretized using finite volumes. The "advection"-like growth is implemented by simple upwind fluxes. The integral is approximated by the midpoint quadrature rule.

• [x] Arbitrary crystal size bins
• [x] Growth
• [x] Growth dispersion
• [x] Analytic Jacobians
• [x] Validation / testing
• [x] Documentation
  • [x] Crystallization
  • [x] HighResKoren
• [x] Add unit tests
  • [x] Create a python script that generates the low resolution reference solutions and should be stored here
  • [x] Numerical reference test for crystallization in a DPFR
  • [x] Numerical reference test for crystallization in a CSTR
  • [x] Open issue to add tests of HighResKoren in another PR (#237)
  • [ ] Rebase onto master once #290 is merged and then increase number of AD directions for test build
  • [x] Python script that creates EOC tests for crystallization in a CSTR and DPFR (basically the ones from the first paper, two settings there (one CSTR and one DPFR) should be enough)
• [x] Cleanup history to separate HighResKoren and Crystallization features
• [x] Rebase / resolving conflicts
• [x] include Sam as co-author in the corresponding squashed commits (in commit description add "Co-authored by: Samuel Leweke [email protected]")
• [x] was comment https://github.com/modsim/CADET/pull/112#issuecomment-1278844645 addressed ?

[sleweke]

I've moved the computation of the WENO coefficients from the residualLiquidImpl() to outside the class. As the coefficients only depend on the _binSizes, we can compute them once at configuration time and store them. This should have multiple advantages:

• Required stack size decreases
• Computation time decreases due to caching the values instead of recomputing them

Of course, I have not tested the changes. Sorry! You may want to make sure that I didn't break anything.

Another thing that caught my eye: Flux through left and right face are computed separately. But don't we have

flux right face of cell i == flux left face of cell i+1

So the idea is to use the flux value from the previous cell's right face as the flux through the current cell's left face. Then update the variable to the flux through the current cell's right face. This would again save a lot of computations.

[schmoelder]

@WFlynnZ could you please give an update on the current status of this project.

• What's missing?
• Do you have an estimate when we can merge a first version of this?

[WFlynnZ]

@schmoelder I think it's pretty much ready. I guess we can merge it when the ms is accepted?

[schmoelder]

I added some comments to the code that need to be addressed before merging.

Also, I couldn't find unit tests for the crystallization module; please add some. Same goes for documentation.

Also also, we need to rebase first, resolve conflicts and get the CI running.

[schmoelder]

I guess we can merge it when the ms is accepted?

It would be even better to merge before publication because then you can simply refer to the CADET master branch instead of a feature branch which will be deleted at some point.

[WFlynnZ]

What is the unit test you mentioned? Isn't the ms the document?

[schmoelder]

Isn't the ms the document?

I assume you mean the manuscript?

What is the unit test you mentioned?

A unit test is some piece of code that is run after building to verify the code does what it's supposed to. E.g. https://github.com/modsim/CADET/blob/master/test/ReactionModelTests.cpp This is good practice in software development and a minimum requirement for merging the PR.

[schmoelder]

Hey @WFlynnZ we're currently cleaning up our project management pipeline and were wondering if you could update us on the current status of the crystallization module. What's missing and when can we expect this to be finished?

Hope all is well!

[WFlynnZ]

I think last time @jbreue16 and I checked we are pretty much all set (for part I, part II will be in a different PR) for residual and jacobian tests for all terms implemented (I need to double check the HR Koren tests). @jbreue16 mentioned he has another idea on how we compare the test result with no analytical solutions and clean up the code to wrap things up? @jbreue16 Are we still on top of that?

[github-actions[bot]]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[jbreue16]

Ive rebased the branch, resolved some conflicts and squahsed the commits into two. I've created a backup branch with the old commits here

@WFlynnZ can you make sure that the crystallization code still works by simply running one or two of your simulations. @WFlynnZ you also need to sign the contributors license agreement as described above by the github bot.

I'll go through the documentation and tests to see if I can update/fix some things, some of which were already mentioned in the above conversation. Flynn and I will meet next week, discuss final changes and hopefully and finally merge the branch into master.

[WFlynnZ]

• [x] The CADET maintainers know my real name.

At least one of the following two applies:

• [x] The CADET maintainers know my current employer.

• [x] I am not making this contribution on behalf of my current employer.

[WFlynnZ]

I have read the CLA Document and I hereby sign the CLA.

[WFlynnZ]

Ive rebased the branch, resolved some conflicts and squahsed the commits into two. I've created a backup branch with the old commits here

@WFlynnZ can you make sure that the crystallization code still works by simply running one or two of your simulations.

I checked one simulation, it still works

@WFlynnZ you also need to sign the contributors license agreement as described above by the github bot.

Signed

[schmoelder]

It would be great if we could finalize this PR s.t. we can include it in v5.0.0 of CADET.

What's currently blocking the merge?

[jbreue16]

Thanks to all contributors, the crystallization module will be part of the version 5 release :tada: