Skip to content

dibin reduction#16

Merged
ThomasHowarth merged 38 commits intopush_branchfrom
jf_dibin_reduction
Jul 15, 2025
Merged

dibin reduction#16
ThomasHowarth merged 38 commits intopush_branchfrom
jf_dibin_reduction

Conversation

@JustinFreiberger
Copy link
Copy Markdown

@JustinFreiberger JustinFreiberger commented Mar 25, 2025

Reduced the size of the "matrix" storing the diffcoefs for the mechanisms. Therefore, adjusted the calculations in ceptr and their usage in Simple.
Beware: Mechanism files need to be reconverted in order to work with the optimized calculations.
This change reduces memory usage and the computational load related to diffusion coefficients.
The calculations are approximately 42% faster than in the previous version, depending on the number of species used in the simulation.

@JustinFreiberger JustinFreiberger marked this pull request as draft March 26, 2025 09:10
@JustinFreiberger JustinFreiberger changed the base branch from development to push_branch April 14, 2025 10:44
@JustinFreiberger JustinFreiberger marked this pull request as ready for review April 17, 2025 17:04
@JustinFreiberger
Copy link
Copy Markdown
Author

Note on the SRK commit. It was just an experiment that I changed back to its original state. Did'nt state that smartly withing my commits

Copy link
Copy Markdown

@terencelehmann terencelehmann left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! I have added a few comments to address before we can merge. Additionally, we should (as you mentioned in the pull request) remake all mechanisms.

terencelehmann
terencelehmann previously approved these changes May 13, 2025
@terencelehmann terencelehmann dismissed their stale review May 13, 2025 15:54

Unclear if tests were conducted with the correct code version.

@JustinFreiberger
Copy link
Copy Markdown
Author

Notes on testing with BrukeITV
Start the first step on development, that way you will only need to remake Burke once.

  1. Firstly you need to remake the BurkeITV mechanism with the convert script within, as this is not part of the repo.
  2. Afterwards you need to compile against this branch in PeleAnalysis/Src/ModelSpecificAnalysis
    -> make sure that you are compiling in 2D and change the chemistry model and MPI
  3. Submit a plot with the newly compiled "plotTransportCoeff.2d.gnu.MPI.ex"
  4. Make two plot files, one being ran with the plotTransportCoeff from this branch, other against development
    For more precise comparison to development
    In order to check for Errors, you need to use a tool. A simple "diff" will not help, as plotfies contain machine error
    Checkout in PeleAnalysis on the branch "jf_dibin_reduction". Now you'll be able to compile my tool called "comparePlts"
    Executing this tool takes in two filenames and a variable, for BurkeITV you can use rhoD(H2). The tool will generate a plotfile, that shows the numerical differences of the variable between the provided plot files.
    2D can be usually run frontend.
    The Outfile will show noise, which originates from the machine error. Seeing regions of higher magnitude (>e15) will be concerning. Otherwise it will be fine

Copy link
Copy Markdown

@ThomasHowarth ThomasHowarth left a comment

Choose a reason for hiding this comment

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

Now looking good - happy to push through

@terencelehmann terencelehmann self-requested a review July 3, 2025 10:05
Copy link
Copy Markdown

@terencelehmann terencelehmann left a comment

Choose a reason for hiding this comment

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

We still need to include the implementation for the SRK parts. Since we changed the layout of the COFD in ceptr, SRK would not work. Probably it is also good to test this part in some way.

@ThomasHowarth ThomasHowarth self-requested a review July 3, 2025 11:13
@JustinFreiberger
Copy link
Copy Markdown
Author

I had to implement a new idx, as the parm->Upsilon matrix is of different order, still using it's symmetry to reduce iterations

@ThomasHowarth
Copy link
Copy Markdown

@terencelehmann if you can approve, I'll merge.

@ThomasHowarth ThomasHowarth merged commit 0d4905e into push_branch Jul 15, 2025
@ThomasHowarth ThomasHowarth deleted the jf_dibin_reduction branch August 5, 2025 08:04
@ThomasHowarth ThomasHowarth restored the jf_dibin_reduction branch August 5, 2025 08:04
@ThomasHowarth ThomasHowarth deleted the jf_dibin_reduction branch August 5, 2025 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants