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

Perf improvements for floating point math #852

Merged
merged 3 commits into from
Jul 15, 2024
Merged

Perf improvements for floating point math #852

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
555ca7e
Perf improvements for floating point math
heshpdx Jul 13, 2024
dc62ea9
Fix formatting in constants.h
heshpdx Jul 13, 2024
e570b03
Formatting fix
heshpdx Jul 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions src/h3lib/include/constants.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,37 +31,43 @@
#endif

/** 2.0 * PI */
#define M_2PI 6.28318530717958647692528676655900576839433L
#define M_2PI 6.28318530717958647692528676655900576839433

/** pi / 180 */
#define M_PI_180 0.0174532925199432957692369076848861271111L
#define M_PI_180 0.0174532925199432957692369076848861271111
/** 180 / pi */
#define M_180_PI 57.29577951308232087679815481410517033240547L
#define M_180_PI 57.29577951308232087679815481410517033240547

/** threshold epsilon */
#define EPSILON 0.0000000000000001L
#define EPSILON 0.0000000000000001
/** sqrt(3) / 2.0 */
#define M_SQRT3_2 0.8660254037844386467637231707529361834714L
#define M_SQRT3_2 0.8660254037844386467637231707529361834714
/** sin(60') */
#define M_SIN60 M_SQRT3_2
/** 1/sin(60') **/
#define M_RSIN60 1.1547005383792515290182975610039149112953

/** one third **/
#define M_ONETHIRD 0.333333333333333333333333333333333333333

/** rotation angle between Class II and Class III resolution axes
* (asin(sqrt(3.0 / 28.0))) */
#define M_AP7_ROT_RADS 0.333473172251832115336090755351601070065900389L
#define M_AP7_ROT_RADS 0.333473172251832115336090755351601070065900389

/** sin(M_AP7_ROT_RADS) */
#define M_SIN_AP7_ROT 0.3273268353539885718950318L
#define M_SIN_AP7_ROT 0.3273268353539885718950318

/** cos(M_AP7_ROT_RADS) */
#define M_COS_AP7_ROT 0.9449111825230680680167902L
#define M_COS_AP7_ROT 0.9449111825230680680167902

/** earth radius in kilometers using WGS84 authalic radius */
#define EARTH_RADIUS_KM 6371.007180918475L
#define EARTH_RADIUS_KM 6371.007180918475

/** scaling factor from hex2d resolution 0 unit length
* (or distance between adjacent cell center points
* on the plane) to gnomonic unit length. */
#define RES0_U_GNOMONIC 0.38196601125010500003L
#define RES0_U_GNOMONIC 0.38196601125010500003
#define INV_RES0_U_GNOMONIC 2.61803398874989588842
heshpdx marked this conversation as resolved.
Show resolved Hide resolved

/** max H3 resolution; H3 version 1 has 16 resolutions, numbered 0 through 15 */
#define MAX_H3_RES 15
Expand Down
2 changes: 1 addition & 1 deletion src/h3lib/lib/coordijk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ void _hex2dToCoordIJK(const Vec2d *v, CoordIJK *h) {
a2 = fabsl(v->y);

// first do a reverse conversion
x2 = a2 / M_SIN60;
x2 = a2 * M_RSIN60;
x1 = a1 + x2 / 2.0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just spotted this. I'm not sure if it matters since powers of two are quick anyway, but I figured I would document that we could change it to x1 = a1 + x2 * 0.5;
Or since this is the only usage of M_RSIN60, just craft a M_RSIN60_DIV_BY_2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I confirmed that the same assembly is produced.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If the same assembly is produced it sounds like it's fine to leave as-is because compiler optimizations take care of it for us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, any compiler at -O1 or higher opt figures it out.


// check if we have the center of a hex
Expand Down
11 changes: 6 additions & 5 deletions src/h3lib/lib/faceijk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@
#include "latLng.h"
#include "vec3d.h"

/** square root of 7 */
#define M_SQRT7 2.6457513110645905905016157536392604257102
/** square root of 7 and inverse square root of 7 */
#define M_SQRT7 2.6457513110645905905016157536392604257102
#define M_RSQRT7 0.37796447300922722721451653623418006081576
heshpdx marked this conversation as resolved.
Show resolved Hide resolved

/** @brief icosahedron face centers in lat/lng radians */
const LatLng faceCenterGeo[NUM_ICOSA_FACES] = {
Expand Down Expand Up @@ -446,12 +447,12 @@ void _hex2dToGeo(const Vec2d *v, int face, int res, int substrate, LatLng *g) {
double theta = atan2(v->y, v->x);

// scale for current resolution length u
for (int i = 0; i < res; i++) r /= M_SQRT7;
for (int i = 0; i < res; i++) r *= M_RSQRT7;

// scale accordingly if this is a substrate grid
if (substrate) {
r /= 3.0;
if (isResolutionClassIII(res)) r /= M_SQRT7;
r *= M_ONETHIRD;
if (isResolutionClassIII(res)) r *= M_RSQRT7;
}

r *= RES0_U_GNOMONIC;
Expand Down
Loading