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.

Sign up for GitHub

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

Jump to bottom

Improve take_while_kld implementation #282

Merged
merged 5 commits into from
Jan 8, 2024
Merged

Improve take_while_kld implementation #282

merged 5 commits into from
Jan 8, 2024

Conversation

nahueespinosa
Copy link
Member

@nahueespinosa nahueespinosa commented Jan 7, 2024 •
edited
Loading

Proposed changes

Improves the take_while_kld view by:

  • Adding overloads to handle particle ranges directly.
  • Removing overloads that take hash ranges.
  • Adding concept checks.

Related to #279.

Type of change

  • 🐛 Bugfix (change which fixes an issue)
  • 🚀 Feature (change which adds functionality)
  • 📚 Documentation (change which fixes or extends documentation)

Checklist

  • Lint and unit tests (if any) pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All commits have been signed for DCO

@nahueespinosa nahueespinosa added enhancement New feature or request cpp Related to C++ code labels Jan 7, 2024
@nahueespinosa nahueespinosa self-assigned this Jan 7, 2024
@nahueespinosa nahueespinosa force-pushed the nahuel/take-while-kld-improvements branch from df62bf4 to 5f9c0b4 Compare January 7, 2024 12:32
@nahueespinosa nahueespinosa changed the title Improve take_while_kld implementation Add forward view and improve take_while_kld implementation Jan 7, 2024
@nahueespinosa nahueespinosa force-pushed the nahuel/take-while-kld-improvements branch from a6b4742 to 2aa34a4 Compare January 7, 2024 14:19
hidmic
hidmic approved these changes Jan 7, 2024
View reviewed changes
beluga/test/beluga/views/test_forward.cpp Outdated Show resolved Hide resolved
beluga/include/beluga/views/take_while_kld.hpp Outdated Show resolved Hide resolved
@nahueespinosa nahueespinosa changed the title Add forward view and improve take_while_kld implementation Improve take_while_kld implementation Jan 8, 2024
@nahueespinosa nahueespinosa force-pushed the nahuel/take-while-kld-improvements branch 2 times, most recently from fc3a4ee to 85e0a3a Compare January 8, 2024 03:30
@nahueespinosa
Improve take_while_kld implementation
9037897 
Add overloads to handle particle ranges directly, improve tests and
add a forward range adaptor to ensure that the kld condition will be
computed once for each element.

Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
@nahueespinosa
Fix static analyzer error
bd4e121 
Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
@nahueespinosa
Add forward view
8833ad4 
Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
@nahueespinosa nahueespinosa force-pushed the nahuel/take-while-kld-improvements branch 2 times, most recently from 8e5e032 to 2331973 Compare January 8, 2024 12:33
@nahueespinosa
Remove forward view
51773a8 
Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
@nahueespinosa nahueespinosa force-pushed the nahuel/take-while-kld-improvements branch from 2331973 to 2cd9ce8 Compare January 8, 2024 12:34
@nahueespinosa
Remove overload that takes two ranges
16f7c17 
It had a major bug where the hash would be computed from one set of
particles and paired with a different set of random particles.

Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
@nahueespinosa nahueespinosa force-pushed the nahuel/take-while-kld-improvements branch from 2cd9ce8 to 16f7c17 Compare January 8, 2024 13:02
@nahueespinosa nahueespinosa merged commit df65342 into main Jan 8, 2024
8 checks passed
@nahueespinosa nahueespinosa deleted the nahuel/take-while-kld-improvements branch January 8, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hidmic hidmic hidmic approved these changes

@glpuga glpuga Awaiting requested review from glpuga

Assignees

@nahueespinosa nahueespinosa

Labels
cpp Related to C++ code enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nahueespinosa @hidmic