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

Add EOC tests for aggregation and fragmentation in crystallization #44

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

WFlynnZ
Copy link
Contributor

@WFlynnZ WFlynnZ commented Jan 13, 2025

This PR implements EOC tests for aggregation and fragmentation equations.

  • Add model setups for all logical combinations of the equations
  • Add EOC evaluation code
  • Run verification code
    • Wait for modularization of the code in CADET-Core, see #CADET-Core/241
    • Do a test run, then merge and rerun, upload output to the output repo

@WFlynnZ WFlynnZ requested a review from jbreue16 January 13, 2025 05:16
@WFlynnZ WFlynnZ self-assigned this Jan 13, 2025
@jbreue16 jbreue16 changed the title Add templates for final tests for aggregation and fragmentation Add EOC tests for aggregation and fragmentation in crystallization Jan 13, 2025
@jbreue16 jbreue16 added the enhancement New feature or request label Jan 13, 2025
@jbreue16
Copy link
Contributor

Thanks Flynn !
Do we also have a case where aggregation + breakage + DPFR transport happens? Or was that not analyzed in the paper or did I oversee it in the script here or is there something speaking against considering this case?

@WFlynnZ
Copy link
Contributor Author

WFlynnZ commented Jan 14, 2025

Hi Jan, we actually have aggregation+breakage+DPFR in our paper, but I did not include it here because I thought we have 1. aggregation and breakage, separately, 2. aggregation and breakage combined, 3. aggregation+DPFR and breakage+DPFR, separately, if these tests all pass, especially we have 2 and 3 passed, what could go wrong if we do aggregation+breakage+DPFR, seems somewhat redundant to me. But I can certainly add if you think it is necessary.

@jbreue16
Copy link
Contributor

Hm I see.. I think it would be best to have tests for all combinations listed in your comment on the core side. Theres always things that can go wrong with implementation unexpectedly

WFlynnZ and others added 3 commits January 15, 2025 14:00
mpmath is used to compute reference solutions with enhanced precision
for the crystallization fragmentation module.
@jbreue16 jbreue16 force-pushed the add_aggregation_fragmentation_tests branch from fbf9ec4 to 06e0e40 Compare January 15, 2025 13:00
@WFlynnZ
Copy link
Contributor Author

WFlynnZ commented Jan 19, 2025

Okay sounds good, I just added another case covering aggregation+fragmentation in a DPFR tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants