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

Shouldn't Tensor, Matrix and Vector share more code? #24

Closed
felix91gr opened this issue May 24, 2017 · 7 comments
Closed

Shouldn't Tensor, Matrix and Vector share more code? #24

felix91gr opened this issue May 24, 2017 · 7 comments

Comments

@felix91gr
Copy link
Contributor

When you see T, M and V, they appear to be almost the same thing, if you judge them by their properties:

@Tensor
    /// Number of elements in the tensor.
    public let count: Int

    /// Number of elements in each dimension of the tensor.
    public var size: [Int]

    /// Data contained in tensor in row-major order.
    public var data: [T]

    /// Optional name of tensor (e.g., for use in display).
    public var name: String?

    /// Determine whether to show name when displaying tensor.
    public var showName: Bool

    /// Formatter to be used in displaying tensor elements.
    public var format: NumberFormatter
@Matrix
    /// Number of elements in the matrix.
    public let count: Int

    /// Number of [rows, columns] in the matrix.
    public var size: [Int]
    public var rows: Int { return self.size[0] }
    public var columns: Int { return self.size[1] }

    /// Data contained in matrix in row-major order.
    public var data: [T]

    /// Optional name of matrix (e.g., for use in display).
    public var name: String?

    /// Determine whether to show name when displaying matrx.
    public var showName: Bool

    /// Formatter to be used in displaying matrix elements.
    public var format: NumberFormatter    

@Vector
    /// Number of elements in vector.
    public let count: Int

    /// Data contained in vector.
    public var data: [T]

    /// Optional name of vector for use in display
    public var name: String?

    /// Determine whether to show name when displaying matrx.
    public var showName: Bool

    /// Formatter to be used in displaying matrix elements.
    public var format: NumberFormatter

For better maintainability, shouldn't they share more code? By means of a protocol, for example. This would help with not only maintenance, it would also help with testing and functions by joining together test cases and functions that are basically the same across types (see the mean function for example).

Btw, I like this project because it actually works in Linux 😄 I wanna help with this in my spare time

@philipce
Copy link
Owner

I think you're absolutely right! These should totally be refactored to adopt a common protocol. Thoughts on a protocol name? Something like NDArray or NiftyArray? Not in love with either...

And I'm happy to have another person interested in the project! It's good to see more people using Swift in Linux. The project has kind of taken a back seat for me the last couple months but I'm stoked to get back on it.

@felix91gr
Copy link
Contributor Author

felix91gr commented May 24, 2017

🤔 The "correct" or "by definition" name would be Tensor, right? Because Matrix and Vector are also Tensors. But that, I think, can't be. Even if we use protocol extensions to provide default implementations, I don't think it's possible to have Tensor be only a protocol and also use it as a struct for Tensor operations.

(I had to mention that idea to be at peace with myself) 😄

Now, leaving it aside... what about calling it TensorProtocol? Or TensorBody? Something that describes what's inside :)

@philipce
Copy link
Owner

philipce commented May 24, 2017 via email

@felix91gr
Copy link
Contributor Author

Haha, nice! We're set then :)

Want me to start a branch and we go from there?
Or a fork, and you give me feedback as I go c:

@philipce
Copy link
Owner

Yeah, sounds good! I've been working on some stuff in a separate project that I need to clean up and integrate here (some pandas type stuff and the beginnings of an optimization toolbox), so that's my main goal now... but if I can be of assistance in any way let me know!

@felix91gr
Copy link
Contributor Author

I can start working on this! (Finally)
The exam period ended and I went on a short trip to the US.

Now I’m back!
There are two things I want to work on: the TensorProtocol and the property testing (to augment the current tests).

I think the protocol (and subsequent refactoring) should come first. Right? The tests I think, are easier. But the structs should be fully defined before we improve the tests. Otherwise we would possibly be writing them again 🤔

Sidenote?: I haven’t yet learned what I wanted to learn about Tensors (I was too busy with the exams), so I’ll take it slowly as I learn more. Can I ask you about Tensors as I learn about them? (If you can’t, don’t worry. I have StackExchange as plan B 😊)

@felix91gr
Copy link
Contributor Author

Closing this one, as #29 took its role :D

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

No branches or pull requests

2 participants