Skip to content

Commit

Permalink
speeded up slice creation by omission of min/max calls
Browse files Browse the repository at this point in the history
Direct if-statements are 4-5 times faster since we
can omit the min+max calls

Signed-off-by: Nick Papior <nickpapior@gmail.com>
  • Loading branch information
zerothi committed Oct 16, 2024
1 parent 1511a47 commit c522040
Showing 1 changed file with 31 additions and 4 deletions.
35 changes: 31 additions & 4 deletions src/sisl/_core/geometry.py
Original file line number Diff line number Diff line change
Expand Up @@ -3760,11 +3760,38 @@ def sphere_grid_index(grid, center, R):

cmin = corners_i.min(axis=0)
cmax = corners_i.max(axis=0) + 1

sh = grid.shape
rx = slice(min(max(cmin[0], 0), sh[0]), min(max(cmax[0], 0), sh[0]))
ry = slice(min(max(cmin[1], 0), sh[1]), min(max(cmax[1], 0), sh[1]))
rz = slice(min(max(cmin[2], 0), sh[2]), min(max(cmax[2], 0), sh[2]))

# direct if-statements are 4-5 times faster than min+max
if cmin[0] < 0:
cmin[0] = 0
elif sh[0] < cmin[0]:
cmin[0] = sh[0]
if cmin[1] < 0:
cmin[1] = 0
elif sh[1] < cmin[1]:
cmin[1] = sh[1]
if cmin[2] < 0:
cmin[2] = 0
elif sh[2] < cmin[2]:
cmin[2] = sh[2]

if cmax[0] < 0:
cmax[0] = 0
elif sh[0] < cmax[0]:
cmax[0] = sh[0]
if cmax[1] < 0:
cmax[1] = 0
elif sh[1] < cmax[1]:
cmax[1] = sh[1]
if cmax[2] < 0:
cmax[2] = 0
elif sh[2] < cmax[2]:
cmax[2] = sh[2]

rx = slice(cmin[0], cmax[0])
ry = slice(cmin[1], cmax[1])
rz = slice(cmin[2], cmax[2])

indices = np.mgrid[rx, ry, rz].reshape(3, -1).T

Expand Down

4 comments on commit c522040

@pfebrer
Copy link
Contributor

Choose a reason for hiding this comment

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

But this makes it very hard to read, is the performance gain significant?

@zerothi
Copy link
Owner Author

Choose a reason for hiding this comment

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

I know, it kind of looks horrible, it is around 5 times faster (each min/max invocation).

I thought this was an ok thing, what I forgot to benchmark against was numpy calls.. I just suspected the array creation to be too large.

@pfebrer
Copy link
Contributor

Choose a reason for hiding this comment

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

But is the timing relevant with respect to the whole orbital values computing function?

@zerothi
Copy link
Owner Author

Choose a reason for hiding this comment

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

I guess you are right... I would assume for coarse grids with bulk things this might be important...

Please sign in to comment.