Skip to content

Commit 91cee09

Browse files
authored
Revert "Removed AlignedArray (#1657)" (#1838)
This reverts commit 72ec121. A bug was detected that resulted in non-deterministic calculation, since the underlying C++ code was written in a way apparently that required alignment to produce consistent results, so of course just removing the alignment and calling an only slightly modified algorithm compromised determinism, resulting in test failure for RFF in particular.
1 parent 435a63b commit 91cee09

File tree

17 files changed

+1699
-809
lines changed

17 files changed

+1699
-809
lines changed
Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
using Microsoft.ML.Runtime.Internal.CpuMath.Core;
66
using System;
77

8-
namespace Microsoft.ML.Runtime.FactorizationMachine
8+
namespace Microsoft.ML.Runtime.Internal.CpuMath
99
{
1010
using Float = System.Single;
1111

@@ -17,6 +17,7 @@ namespace Microsoft.ML.Runtime.FactorizationMachine
1717
///
1818
/// The ctor takes an alignment value, which must be a power of two at least sizeof(Float).
1919
/// </summary>
20+
[BestFriend]
2021
internal sealed class AlignedArray
2122
{
2223
// Items includes "head" items filled with NaN, followed by _size entries, followed by "tail"
@@ -109,12 +110,102 @@ public Float this[int index]
109110
}
110111
}
111112

113+
public void CopyTo(Span<Float> dst, int index, int count)
114+
{
115+
Contracts.Assert(0 <= count && count <= _size);
116+
Contracts.Assert(dst != null);
117+
Contracts.Assert(0 <= index && index <= dst.Length - count);
118+
Items.AsSpan(_base, count).CopyTo(dst.Slice(index));
119+
}
120+
121+
public void CopyTo(int start, Span<Float> dst, int index, int count)
122+
{
123+
Contracts.Assert(0 <= count);
124+
Contracts.Assert(0 <= start && start <= _size - count);
125+
Contracts.Assert(dst != null);
126+
Contracts.Assert(0 <= index && index <= dst.Length - count);
127+
Items.AsSpan(start + _base, count).CopyTo(dst.Slice(index));
128+
}
129+
130+
public void CopyFrom(ReadOnlySpan<Float> src)
131+
{
132+
Contracts.Assert(src.Length <= _size);
133+
src.CopyTo(Items.AsSpan(_base));
134+
}
135+
136+
public void CopyFrom(int start, ReadOnlySpan<Float> src)
137+
{
138+
Contracts.Assert(0 <= start && start <= _size - src.Length);
139+
src.CopyTo(Items.AsSpan(start + _base));
140+
}
141+
142+
// Copies values from a sparse vector.
143+
// valuesSrc contains only the non-zero entries. Those are copied into their logical positions in the dense array.
144+
// rgposSrc contains the logical positions + offset of the non-zero entries in the dense array.
145+
// rgposSrc runs parallel to the valuesSrc array.
146+
public void CopyFrom(ReadOnlySpan<int> rgposSrc, ReadOnlySpan<Float> valuesSrc, int posMin, int iposMin, int iposLim, bool zeroItems)
147+
{
148+
Contracts.Assert(rgposSrc != null);
149+
Contracts.Assert(valuesSrc != null);
150+
Contracts.Assert(rgposSrc.Length <= valuesSrc.Length);
151+
Contracts.Assert(0 <= iposMin && iposMin <= iposLim && iposLim <= rgposSrc.Length);
152+
153+
// Zeroing-out and setting the values in one-pass does not seem to give any perf benefit.
154+
// So explicitly zeroing and then setting the values.
155+
if (zeroItems)
156+
ZeroItems();
157+
158+
for (int ipos = iposMin; ipos < iposLim; ++ipos)
159+
{
160+
Contracts.Assert(posMin <= rgposSrc[ipos]);
161+
int iv = _base + rgposSrc[ipos] - posMin;
162+
Contracts.Assert(iv < _size + _base);
163+
Items[iv] = valuesSrc[ipos];
164+
}
165+
}
166+
112167
public void CopyFrom(AlignedArray src)
113168
{
114169
Contracts.Assert(src != null);
115170
Contracts.Assert(src._size == _size);
116171
Contracts.Assert(src._cbAlign == _cbAlign);
117172
Array.Copy(src.Items, src._base, Items, _base, _size);
118173
}
174+
175+
public void ZeroItems()
176+
{
177+
Array.Clear(Items, _base, _size);
178+
}
179+
180+
public void ZeroItems(int[] rgposSrc, int posMin, int iposMin, int iposLim)
181+
{
182+
Contracts.Assert(rgposSrc != null);
183+
Contracts.Assert(0 <= iposMin && iposMin <= iposLim && iposLim <= rgposSrc.Length);
184+
Contracts.Assert(iposLim - iposMin <= _size);
185+
186+
int ivCur = 0;
187+
for (int ipos = iposMin; ipos < iposLim; ++ipos)
188+
{
189+
int ivNextNonZero = rgposSrc[ipos] - posMin;
190+
Contracts.Assert(ivCur <= ivNextNonZero && ivNextNonZero < _size);
191+
while (ivCur < ivNextNonZero)
192+
Items[_base + ivCur++] = 0;
193+
Contracts.Assert(ivCur == ivNextNonZero);
194+
// Skip the non-zero element at ivNextNonZero.
195+
ivCur++;
196+
}
197+
198+
while (ivCur < _size)
199+
Items[_base + ivCur++] = 0;
200+
}
201+
202+
// REVIEW: This is hackish and slightly dangerous. Perhaps we should wrap this in an
203+
// IDisposable that "locks" this, prohibiting GetBase from being called, while the buffer
204+
// is "checked out".
205+
public void GetRawBuffer(out Float[] items, out int offset)
206+
{
207+
items = Items;
208+
offset = _base;
209+
}
119210
}
120211
}

0 commit comments

Comments
 (0)