Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added normalization feature for matrices and fixed issue where ValueArraySlice could not be converted property to a ValueArray #56

Merged
merged 1 commit into from Aug 2, 2016

Conversation

Somnibyte
Copy link

@Somnibyte Somnibyte commented Aug 1, 2016

I'm currently working on an implementation of Lasso Regression for a machine learning framework I'm working on and I stumbled upon an issue where UpSurge didn't have the ability to normalize my feature vectors. So I added the ability to normalize an entire matrix. I had stumbled upon an issue recently where whenever I tried to convert a ValueArraySlice to a ValueArray, the ValueArray class automatically adopts it's own step field rather than utilizing the original step field of the ValueArraySlice.

Here is an example:

let inputMatrix = Matrix<Float>(x)
let vectorSlice = inputMatrix .column(i)  // [3,4]
// The base of 'vectorSlice' is [3,5,8,4,12,15]

Now if I were to do the following I would get [3,5] instead of a ValueArray that has the elements 3,4 like the slice originally had:

let vectorSlice = ValueArray(inputMatrix .column(i)) // [3,5]
This is becuase within the ValueArray class the step size provided within the class, which is set to 1, is always adopted by this method:

Line 37 of ValueArray class:

    public var step: Index {
        return 1
    }


Problematic area (Line 88):

    /// Construct a ValueArray from contiguous memory
    public required init<C: LinearType where C.Element == Element>(_ values: C) {
        mutablePointer = UnsafeMutablePointer<Element>.alloc(values.count)
        capacity = values.count
        count = values.count
        values.withUnsafeBufferPointer { pointer in
            for i in 0..<count {
                mutablePointer[i] = pointer[values.startIndex + i * step]
            }
        }
    }

Above you see that in this line here, we are only using the step provided by the class. Instead we need the step provided by the ValueArraySlice (which in my example originally had a step of 3):

Original:
mutablePointer[i] = pointer[values.startIndex + i * step]

To fix this I use the 'values' parameters step field.

mutablePointer[i] = pointer[values.startIndex + i * values.step]

The normalize functions I implemented work as expected and have been tested.

…d issue where ValueArraySlice does not convert to a ValueArray due to not adopting the ValueArraySlice's step field
@Somnibyte Somnibyte changed the title Added normalization to matrixes and fixed issue where ValueArraySlice could not be converted property to a ValueArray Added normalization feature for matrices and fixed issue where ValueArraySlice could not be converted property to a ValueArray Aug 1, 2016
@aidangomez
Copy link
Collaborator

Nice catch!

@Somnibyte
Copy link
Author

Thanks @aidangomez! 😄

@alejandro-isaza alejandro-isaza merged commit 63defcb into alejandro-isaza:master Aug 2, 2016
@alejandro-isaza
Copy link
Owner

Thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants