Skip to content

Optimize memory allocation during burn-in phase (#180)#460

Open
Omar-Abdo1 wants to merge 3 commits intoGeomScale:developfrom
Omar-Abdo1:fix-burn-in-memory
Open

Optimize memory allocation during burn-in phase (#180)#460
Omar-Abdo1 wants to merge 3 commits intoGeomScale:developfrom
Omar-Abdo1:fix-burn-in-memory

Conversation

@Omar-Abdo1
Copy link
Copy Markdown

Hello @TolisChal @vissarion,

I was exploring the codebase and noticed Issue #180 regarding the inefficient memory handling during the burn-in phase. Currently, the loops store unstable points in randPoints only to clear them immediately after the nburns block.

Changes made:
To resolve this and prevent unnecessary memory allocation, I introduced a NullWalkPolicy struct.

  • This policy bypasses memory allocation completely by silently discarding the point.
  • I replaced push_back_policy (and equivalent policies) with NullWalkPolicy across all nburns > 0 blocks in include/sampling.hpp.
  • Removed the now-redundant randPoints.clear() calls, as the vector remains empty during burn-in.

The code compiles locally without issues and all C++ algorithms continue to function correctly. Let me know if you would like any adjustments or further optimizations to this approach!

Fixes #180

Copy link
Copy Markdown
Member

@vissarion vissarion 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 this PR. I have minor comments.

Comment thread include/sampling/sampling.hpp Outdated
Polytope &P,
RandomNumberGenerator &rng,
const unsigned int &walk_len,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is not needed

Comment thread include/volume/sampling_policies.hpp Outdated
};

struct NullWalkPolicy{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please fix indentation

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the review! I have fixed the indentation of NullWalkPolicy , I also removed the stress test files that were accidentally included. Please let me know if there are any further changes needed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have addressed all review comments — fixed the indentation , I noticed some cmake-clang checks are failing on certain compilers. Could you let me know if these are pre-existing CI issues, or if there is something I should look into on my end?

@Omar-Abdo1 Omar-Abdo1 requested a review from vissarion March 14, 2026 03:35
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.

Inefficient handling of burn in phase in sampling functions

2 participants