Skip to content

Conversation

@michaellass
Copy link
Contributor

Until now sqrt(n) was missed as a factor for odd square numbers n. This lead to suboptimal results of MPI_Dims_create for input numbers like 9, 25, 49, ... Fix the results by including sqrt(n) in the search for factors.

Refs: #7186

Until now sqrt(n) was missed as a factor for odd square numbers n. This
lead to suboptimal results of MPI_Dims_create for input numbers like 9,
25, 49, ... Fix the results by including sqrt(n) in the search for
factors.

Refs: open-mpi#7186

Signed-off-by: Michael Lass <[email protected]>
@ompiteam-bot
Copy link

Can one of the admins verify this patch?

@awlauria
Copy link
Contributor

ok to test

@michaellass
Copy link
Contributor Author

michaellass commented Nov 22, 2019

Tested by creating 2D dims for numbers 1-99:

         Before         After  
1:     1       1      1      1
2:     2       1      2      1
3:     3       1      3      1
4:     2       2      2      2
5:     5       1      5      1
6:     3       2      3      2
7:     7       1      7      1
8:     4       2      4      2
9:     9       1      3      3   <---
10:    5       2      5      2
11:    11      1      11     1
12:    4       3      4      3
13:    13      1      13     1
14:    7       2      7      2
15:    5       3      5      3
16:    4       4      4      4
17:    17      1      17     1
18:    6       3      6      3
19:    19      1      19     1
20:    5       4      5      4
21:    7       3      7      3
22:    11      2      11     2
23:    23      1      23     1
24:    6       4      6      4
25:    25      1      5      5   <---
26:    13      2      13     2
27:    9       3      9      3
28:    7       4      7      4
29:    29      1      29     1
30:    6       5      6      5
31:    31      1      31     1
32:    8       4      8      4
33:    11      3      11     3
34:    17      2      17     2
35:    7       5      7      5
36:    6       6      6      6
37:    37      1      37     1
38:    19      2      19     2
39:    13      3      13     3
40:    8       5      8      5
41:    41      1      41     1
42:    7       6      7      6
43:    43      1      43     1
44:    11      4      11     4
45:    9       5      9      5
46:    23      2      23     2
47:    47      1      47     1
48:    8       6      8      6
49:    49      1      7      7   <---
50:    10      5      10     5
51:    17      3      17     3
52:    13      4      13     4
53:    53      1      53     1
54:    9       6      9      6
55:    11      5      11     5
56:    8       7      8      7
57:    19      3      19     3
58:    29      2      29     2
59:    59      1      59     1
60:    10      6      10     6
61:    61      1      61     1
62:    31      2      31     2
63:    9       7      9      7
64:    8       8      8      8
65:    13      5      13     5
66:    11      6      11     6
67:    67      1      67     1
68:    17      4      17     4
69:    23      3      23     3
70:    10      7      10     7
71:    71      1      71     1
72:    12      6      12     6
73:    73      1      73     1
74:    37      2      37     2
75:    15      5      15     5
76:    19      4      19     4
77:    11      7      11     7
78:    13      6      13     6
79:    79      1      79     1
80:    10      8      10     8
81:    9       9      9      9
82:    41      2      41     2
83:    83      1      83     1
84:    12      7      12     7
85:    17      5      17     5
86:    43      2      43     2
87:    29      3      29     3
88:    11      8      11     8
89:    89      1      89     1
90:    10      9      10     9
91:    13      7      13     7
92:    23      4      23     4
93:    31      3      31     3
94:    47      2      47     2
95:    19      5      19     5
96:    12      8      12     8
97:    97      1      97     1
98:    14      7      14     7
99:    11      9      11     9

Copy link
Contributor

@awlauria awlauria left a comment

Choose a reason for hiding this comment

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

LGTM.

@jsquyres
Copy link
Member

@michaellass Awesome! Would you mind opening a few more PRs -- git cherry-pick -x ...thiscommit... to the v3.0.x, v3.1.x, and v4.0.x branches? That will get this fix into the next releases of those series.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants