Skip to content

Conversation

@JoshEliades
Copy link

Added hash algorithm for Coordinate object using XOR, bit shifts and prime multiplication

@nhatdongdang
Copy link
Contributor

I think the hash collisions are still high. You can refer to this code
image

#include <bits/stdc++.h>

std::size_t getHash1(int x, int y, int z)
{
    return (std::hash<int>()(x) * 31) ^
           ((std::hash<int>()(y) * 37) << 8) ^
           ((std::hash<int>()(z) * 41) << 16);
}

std::size_t getHash2(int x, int y, int z)
{
    // Minecraft coordinate bounds
    int lower = -3e7, upper = 3e7;
    size_t base = upper - lower + 1;

    // Make x,y,z non negative
    size_t nx = x - lower, ny = y - lower, nz = z - lower;

    // Use overflow instead of modding
    size_t hash = nx * base * base + ny * base + nz;
    return hash;
}

int main()
{

    std::set<std::size_t> visited1, visited2;

    int startX = 0, endX = 1000;
    int startY = 0, endY = 20;
    int startZ = 0, endZ = 1000;

    for (int x = startX; x <= endX; x++)
    {
        for (int y = startY; y <= endY; y++)
        {
            for (int z = startZ; z <= endZ; z++)
            {
                visited1.insert(getHash1(x, y, z));
                visited2.insert(getHash2(x, y, z));
            }
        }
    }

    int expectedSize = (endX - startX + 1) * (endY - startY + 1) * (endZ - startZ + 1);
    std::cout << "getHash1 collisions: " << expectedSize - (int)visited1.size() << "\n";
    std::cout << "getHash2 collisions: " << expectedSize - (int)visited2.size() << "\n";
}

@JoshEliades
Copy link
Author

Looks good to me, lets go with that then

@rozukke rozukke added enhancement New feature or request next-release To be released next time breaking changes are allowed labels Oct 30, 2024
@rozukke
Copy link
Owner

rozukke commented Oct 30, 2024

Would be great to get a solution with minimal collisions. Additionally, some sort of testing would be nice to make sure this compiles and runs on all systems. In any case, this is delayed until next release.

@rozukke rozukke marked this pull request as draft October 30, 2024 01:39
@rozukke
Copy link
Owner

rozukke commented Dec 5, 2024

@JoshEliades are you willing to write some tests for this implementation? If not maybe @nhatdongdang can do it

@JoshEliades
Copy link
Author

I can write some tests for the implementation

@johnathanchann
Copy link
Contributor

johnathanchann commented Feb 16, 2025

inside local_tests.cpp

 SUBCASE("Test hash no collision"){
        std::set<size_t> hashes;
        std::random_device rd;
        std::mt19937 gen(rd());
        std::uniform_int_distribution<int> dis(-3e7, 3e7);
        std::uniform_int_distribution<int> dis2(-64, 256);

        Coordinate hashing;
        for (int i = 0; i < 100; i++) {
            Coordinate testCoord(dis(gen), dis2(gen), dis(gen));
            size_t hash = hashing(testCoord);
            hashes.insert(hash);
        }
        CHECK_EQ(hashes.size(), 100);
    }

@rozukke
Copy link
Owner

rozukke commented Feb 16, 2025

inside local_tests.cpp

 SUBCASE("Test hash no collision"){
        std::set<size_t> hashes;
        std::random_device rd;
        std::mt19937 gen(rd());
        std::uniform_int_distribution<int> dis(-3e7, 3e7);
        std::uniform_int_distribution<int> dis2(-64, 256);

        Coordinate hashing;
        for (int i = 0; i < 100; i++) {
            Coordinate testCoord(dis(gen), dis2(gen), dis(gen));
            size_t hash = hashing(testCoord);
            hashes.insert(hash);
        }
        CHECK_EQ(hashes.size(), 100);
    }

Please do not do unseeded randomisation in tests

@johnathanchann
Copy link
Contributor

johnathanchann commented Feb 16, 2025

added seed

   SUBCASE("Test hash no collision"){
        const int seed = 12345;
        std::set<size_t> hashes;
        std::mt19937 gen(seed);
        std::uniform_int_distribution<int> dis(-3e7, 3e7);
        std::uniform_int_distribution<int> dis2(-64, 256);
    
        Coordinate hashing;
        for (int i = 0; i < 100; i++) {
            Coordinate testCoord(dis(gen), dis2(gen), dis(gen));
            size_t hash = hashing(testCoord);
            hashes.insert(hash);
        }
        CHECK_EQ(hashes.size(), 100);
    }

@rozukke
Copy link
Owner

rozukke commented Apr 2, 2025

Implemented in #114

@rozukke rozukke closed this Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request next-release To be released next time breaking changes are allowed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants