Question about slice function
Schultzer opened this issue · 4 comments
Hi,
First of all thanks for all this work. I have a question about
Line 425 in b994277
is there a reason why you would use undefined
behavior here?
In the first round base + (i - rowStart) == -1
which result in a undefined
behavior and then skips by one and turns the im = 0
.
if this is the purpose wouldn't it be better to wrap it around an if statement to make it more clear what the desired behavior would be?
Hello,
thank you for your interest,
first, its very clear, the code is wrapped in a small for loop, look what the starting (and ending) value of i
is. just 4 lines above. (421)
for (let i = rowStart; i <= rowEnd; i++) {` // <-- where "i" comes from
// console.log(j, coorJ + i, base + i, r[coorJ + i]);
re[base + i] = r[coorJ + i];
if (this.i && im) {
im[base + (i - rowStart)] = this.i[coorJ + i]; <-- the line of your prev question
}
}
this makes base + (i - rowStart) == 0
The mapping is neccesary because, Not all langauges have zero based indexed arrays/matrices (fortran it is flexible and defaults to 1).
(rowBase and colbase properties)
https://github.com/R-js/blasjs#matrix
If base + (i - rowStart) == 0
is assume to always return 0 on the first round, then I think we have a bug because when I added this line to the loop console.log(`re: base + i = ${base + i} im: base + (i - rowStart) = ${base + (i - rowStart)}`);
it clearly shows that on the first round base + (i - rowStart) = -1
fixtures.matrix_mxn(6, 6).slice(1, 6, 1, 6)
re: base + i = 0 im: base + (i - rowStart) = -1
re: base + i = 1 im: base + (i - rowStart) = 0
re: base + i = 2 im: base + (i - rowStart) = 1
re: base + i = 3 im: base + (i - rowStart) = 2
re: base + i = 4 im: base + (i - rowStart) = 3
re: base + i = 5 im: base + (i - rowStart) = 4
re: base + i = 6 im: base + (i - rowStart) = 5
re: base + i = 7 im: base + (i - rowStart) = 6
re: base + i = 8 im: base + (i - rowStart) = 7
re: base + i = 9 im: base + (i - rowStart) = 8
re: base + i = 10 im: base + (i - rowStart) = 9
re: base + i = 11 im: base + (i - rowStart) = 10
re: base + i = 12 im: base + (i - rowStart) = 11
re: base + i = 13 im: base + (i - rowStart) = 12
re: base + i = 14 im: base + (i - rowStart) = 13
re: base + i = 15 im: base + (i - rowStart) = 14
re: base + i = 16 im: base + (i - rowStart) = 15
re: base + i = 17 im: base + (i - rowStart) = 16
re: base + i = 18 im: base + (i - rowStart) = 17
re: base + i = 19 im: base + (i - rowStart) = 18
re: base + i = 20 im: base + (i - rowStart) = 19
re: base + i = 21 im: base + (i - rowStart) = 20
re: base + i = 22 im: base + (i - rowStart) = 21
re: base + i = 23 im: base + (i - rowStart) = 22
re: base + i = 24 im: base + (i - rowStart) = 23
re: base + i = 25 im: base + (i - rowStart) = 24
re: base + i = 26 im: base + (i - rowStart) = 25
re: base + i = 27 im: base + (i - rowStart) = 26
re: base + i = 28 im: base + (i - rowStart) = 27
re: base + i = 29 im: base + (i - rowStart) = 28
re: base + i = 30 im: base + (i - rowStart) = 29
re: base + i = 31 im: base + (i - rowStart) = 30
re: base + i = 32 im: base + (i - rowStart) = 31
re: base + i = 33 im: base + (i - rowStart) = 32
re: base + i = 34 im: base + (i - rowStart) = 33
re: base + i = 35 im: base + (i - rowStart) = 34
{ rowBase: 1,
colBase: 1,
nrCols: 6,
nrRows: 6,
r:
Float64Array [
1.2629542848807933,
-0.3262333607056494,
1.3297992629225006,
1.2724293214294047,
0.4146414344564082,
-1.5399500419037095,
-0.9285670347135381,
-0.2947204467905602,
-0.005767172747536955,
2.404653388857951,
0.7635934611404596,
-0.7990092489893682,
-1.1476570092363514,
-0.28946157368822334,
-0.29921511789731614,
-0.411510832795067,
0.2522234481561323,
-0.8919211272845686,
0.43568329935571865,
-1.237538421929958,
-0.22426788527830935,
0.37739564598170106,
0.1333363608148414,
0.8041895097449078,
-0.057106774383808755,
0.5036079722337261,
1.085769362145687,
-0.6909538396968303,
-1.2845993538721883,
0.04672617218835198,
-0.23570655643950122,
-0.5428882550102544,
-0.4333103174567822,
-0.6494716467962331,
0.726750747385451,
1.1519117540872 ],
i:
Float64Array [
-0.42951310949188126,
1.2383041008533804,
-0.2793462818542693,
1.7579030898107073,
0.5607460908880562,
-0.4527839725531578,
-0.8320432961178319,
-1.166570547084707,
-1.0655905803882961,
-1.563782051071005,
1.1565369971501793,
0.8320471285723897,
-0.22732869142475534,
0.2661373616721048,
-0.3767027185836281,
2.4413646288945894,
-0.7953391172553718,
-0.054877473711578625,
0.2501413228541527,
0.6182432935662469,
-0.17262350264585732,
-2.2239002740099374,
-1.263614384970583,
0.3587288959713519,
-0.011045478465663564,
-0.9406491626186084,
-0.11582532215695436,
-0.8149687088699175,
0.24226348085968588,
-1.4250983947324998,
0.36594112304921983,
0.2484126488725964,
0.06528818167162072,
0.01915639166027384,
0.2573383771555333,
0 ],
coord: [Function: coord],
colOfEx: [Function: colOfEx],
setCol: [Function: setCol],
upperBand: [Function: upperBand],
lowerBand: [Function: lowerBand],
packedUpper: [Function: packedUpper],
packedLower: [Function: packedLower],
toArr: [Function: toArr],
slice: [Function: slice],
setLower: [Function: setLower],
setUpper: [Function: setUpper],
real: [Function: real],
imaginary: [Function: imaginary] }
I stand corrected, there actually several issues with that piece of code,
the
- -1 is hardcoded default offset of fortran index base "1" that should be changed to
rowBase
- the rowStart needs to be corrected with the rowbase
base + (i - (rowStart - rowBase))
(imaginary part) - the real part is not correct
base + i
should be same as above
Will correct today and do some tests, then release, thanks for finding the issue
version is now 1.0.12
Fixed