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

Fix prime number in voxel hashing #1626

Merged
merged 1 commit into from
Jul 4, 2023
Merged

Fix prime number in voxel hashing #1626

merged 1 commit into from
Jul 4, 2023

Conversation

benemer
Copy link
Contributor

@benemer benemer commented Jun 13, 2023

Hi!

I found that one of the constants used for hashing is not a prime number as claimed in the paper (19349663=41*471943). Note that this is also the case in the referred publication of Teschner et al. [2003].

I replaced it with the next closest prime number which is 19349669, which has been done here as well.

Best
Benedikt

@benemer benemer requested a review from kmuseth as a code owner June 13, 2023 10:48
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 13, 2023 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: benemer / name: Benedikt Mersch (d57bca5)

Idclip
Idclip approved these changes Jun 21, 2023
View reviewed changes
Copy link
Contributor

@Idclip Idclip 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! I'd consider adding a comment akin to this PR description in the code (seeing as it now differs from the paper) but otherwise looks good

@benemer
Copy link
Contributor Author

benemer commented Jun 22, 2023

Thanks for this! I'd consider adding a comment akin to this PR description in the code (seeing as it now differs from the paper) but otherwise looks good

Good point, I just added two comments.

@nachovizzo nachovizzo mentioned this pull request Jun 26, 2023
Merged
@benemer benemer closed this Jun 26, 2023
@benemer
Make all numbers prime
d57bca5 
The prime numbers are modified based on the ACM Transactions on Graphics
paper:  "Real-time 3D reconstruction at scale using voxel hashing"

Signed-off-by: Benedikt Mersch <benedikt.mersch@gmail.com>
@benemer benemer reopened this Jun 26, 2023
kmuseth
kmuseth approved these changes Jun 27, 2023
View reviewed changes
Copy link
Contributor

@kmuseth kmuseth left a comment

Choose a reason for hiding this comment

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

Good catch

@Idclip Idclip merged commit 19710c1 into AcademySoftwareFoundation:master Jul 4, 2023
42 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kmuseth kmuseth kmuseth approved these changes

@Idclip Idclip Idclip approved these changes

@apradhana apradhana Awaiting requested review from apradhana

@jmlait jmlait Awaiting requested review from jmlait

@danrbailey danrbailey Awaiting requested review from danrbailey

@richhones richhones Awaiting requested review from richhones

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benemer @kmuseth @Idclip