Skip to content

Fix memory leak for Soret isothermal boundaries#512

Merged
baperry2 merged 1 commit intoAMReX-Combustion:developmentfrom
baperry2:soret-memory-leak
May 16, 2025
Merged

Fix memory leak for Soret isothermal boundaries#512
baperry2 merged 1 commit intoAMReX-Combustion:developmentfrom
baperry2:soret-memory-leak

Conversation

@baperry2
Copy link
Copy Markdown
Collaborator

@baperry2 baperry2 commented May 16, 2025

Some multifabs for setting up Soret isothermal BCs are allocated with new but never deleted. This causes a memory leak that was noticed by @bssoriano.

ThomasHowarth
ThomasHowarth approved these changes May 16, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

My bad - thanks for this Bruce; looks good.

Just as a note on the Soret stuff, we're working on making a NUM_LITE_SPECIES in the mechanisms and reducing the size of the coefficient MultiFAB which should reduce memory usage, particularly for large mechanisms.

@ThomasHowarth
Copy link
Copy Markdown
Contributor

ThomasHowarth commented May 16, 2025

I'm happy with how it is currently, but we could also use a std::unique_ptr<MultiFab> instead of new and the memory cleanup should be handled for us, right?

@baperry2
Copy link
Copy Markdown
Collaborator Author

baperry2 commented May 16, 2025

There's probably a nicer way to do this with std::unique_ptr, which is what I tried first, but that led to type conversion issues for the vector of arrays of multifab pointers, so I just defaulted to adding delete rather than messing around with function interfaces.

Sounds good on moving NUM_LITE_SPECIES.

@baperry2 baperry2 merged commit 98ba1a5 into AMReX-Combustion:development May 16, 2025
23 checks passed
@baperry2 baperry2 deleted the soret-memory-leak branch May 16, 2025 15:13
malihass added a commit to malihass/PeleLMeX that referenced this pull request May 16, 2025
@malihass
Merge pull request #3 from AMReX-Combustion/development
c6e3093 
delete new allocated multifabs in Soret isothermal (AMReX-Combustion#512)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ThomasHowarth ThomasHowarth ThomasHowarth approved these changes

@mrieth1 mrieth1 Awaiting requested review from mrieth1

@bssoriano bssoriano Awaiting requested review from bssoriano

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@baperry2 @ThomasHowarth