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.

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

Fix compilation errors for rgeoda #5

Merged
merged 5 commits into from
Jun 28, 2023
Merged

Fix compilation errors for rgeoda #5

merged 5 commits into from
Jun 28, 2023

Conversation

lixun910
Copy link
Member

@lixun910 lixun910 commented Jan 1, 2023

No description provided.

@lixun910 lixun910 changed the title Fix centroid function for rgeoda Fix compilation errors for rgeoda Jan 2, 2023
: (unsigned long long)(((unsigned long long)nextLong() >> 32) * n) >> 32);
return r;
return r < 0 ? 0 : r;
Copy link
Member Author

Choose a reason for hiding this comment

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

@lixun910 Here the algorithm will return -1 in some cases, which is not correct. I need to check why this happens. But this should fix the issues that some functions calling nextInt() crashes rgeoda, see GeoDaCenter/rgeoda#39 GeoDaCenter/rgeoda#38 GeoDaCenter/rgeoda#33

@@ -241,7 +241,7 @@ namespace ttmath
/*!
on 64bit platforms one word (uint, sint) will be equal 64bits
*/
typedef unsigned long uint;
typedef unsigned long long uint;
Copy link
Member Author

Choose a reason for hiding this comment

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

@lixun910 I should pay attention to this change:

unsigned long long is guaranteed to be at least 64 bits, regardless of the platform. unsigned long is guaranteed to be at least 32 bits.

Copy link
Member Author

Choose a reason for hiding this comment

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

Anyway, this could be discarded by switching to boost::centroid

@lixun910 lixun910 merged commit d481d71 into main Jun 28, 2023
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.

1 participant