features/110-split_output_randn: Adding the new code for random #114
Conversation
Codecov Report
@@ Coverage Diff @@
## master #114 +/- ##
==========================================
- Coverage 90.34% 90.23% -0.11%
==========================================
Files 20 20
Lines 2735 2735
==========================================
- Hits 2471 2468 -3
- Misses 264 267 +3
Continue to review full report at Codecov.
|
coquelin77
changed the title
heat/core/tests/test_operations.py
Outdated
| self.assertTrue((ht.argmin(random_data,axis=0)._tensor__array == random_data_split.argmin(axis=0)._tensor__array).all()) | ||
| self.assertTrue((ht.argmin(random_data,axis=1)._tensor__array == random_data_split.argmin(axis=1)._tensor__array).all()) | ||
| self.assertIsInstance(ht.argmin(random_data_split,axis=1),ht.tensor) | ||
| # todo: these tests now fail if random_data is split. |
heat/core/tests/test_operations.py
Outdated
|
|
||
| self.assertTrue((ht.max(random_data,axis=0)._tensor__array[0] == random_data_split.max(axis=0)._tensor__array[0]).all()) | ||
| self.assertTrue((ht.max(random_data,axis=1)._tensor__array[0] == random_data_split.max(axis=1)._tensor__array[0]).all()) | ||
| # todo: these tests now fail if random_data is split. |
| @@ -30,7 +30,7 @@ def randn(*args, split=None, comm=MPI_WORLD): | |||
|
|
|||
| Parameters | |||
| ---------- | |||
| d0, d1, …, dn : int, optional | |||
| d0, d1, …, dn : ints, optional | |||
There was a problem hiding this comment.
Implementation of the function is closely related to issue #54. What is the intended behaviour of this function? Output a number of random numbers where each of the split parts is random and not identical to the other split chunks of the other nodes? What will happen if I use the very same seed but a different node count? Will the random tensor differ?
There was a problem hiding this comment.
It looks like torch's randn will not generate the same values for arrays of different sizes. This means that even though the same seed is used, the split tensor will not be the same as a tensor generated only on one process. i.e.
torch.randn((3, 3, 3))[0] != torch.randn((1, 3, 3))One solution to this would be to generate the whole dataset then split it. But this will not scale. I do not see another way to do this.
It must be noted that the matrix generated by the ht.randn with a fixed seed (torch.manual_seed) and a fixed size produces a reproducible result
There was a problem hiding this comment.
In my opinion, we can leave it as proposed for now, however, for the future, I would want to actually have a different behaviour. Consider the following example:
ht.set_seed(1)
ht.randn(100, 5, 3, split=0)
Should always produce the same set of random numbers independent of the utilized nodes (for reproducibility reasons as you mentioned). This is exactly what is requested in issue #54. Obviously, this means that we would have to come up with a pseudo random generator allowing to skip to arbitrary/some fixed positions into the random sequence.
In the proposed fix, we would have a simplified randn() call. It will only provide reproducible results for the exact same node count.
There was a problem hiding this comment.
100% agree. i am trying to figure a way to do this. i will move this discussion to the issue and close this request.
heat/core/tests/test_operations.py
Outdated
|
|
||
| self.assertTrue((ht.min(random_data,axis=0)._tensor__array[0] == random_data_split.min(axis=0)._tensor__array[0]).all()) | ||
| self.assertTrue((ht.min(random_data,axis=1)._tensor__array[0] == random_data_split.min(axis=1)._tensor__array[0]).all()) | ||
| # todo: these tests now fail if random_data is split. |
see issue 110.
heads up @ClaudiaComito i mentioned this in the issue as well but this will change the tests for argmin, min, and max. This is because the random numbers generated on separate processes are not equal but they are the same every run.