Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Add System.Numerics.Matrices #118

Merged
merged 1 commit into from
Jul 16, 2015
Merged

Add System.Numerics.Matrices #118

merged 1 commit into from
Jul 16, 2015

Conversation

whoisj
Copy link
Contributor

@whoisj whoisj commented Jul 7, 2015

Adds System.Numerics.Matrices

Based on a text-template which can support any sized matrix

Created up to 4x4 matrices initially (up for discussion)

Adds tests for each matrix type

Tests are text template generated, therefore easy to add to each matrix type

Adds a .gitignore to the System.Numerics.Matrices directory to ignore TT temp files

@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many git push -f later and I still do not know what I've done to edit this line. 😖

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git diff on the commandline is helpful for this:

image

Looks like you changed the BOM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akoeplinger Thanks for that. I'm using Git for Windows, and git diff HEAD~..HEAD -- src/NuGet.Config wasn't showing the BOM difference. 😒

Based on a text-template which can support any sized matrix

Created up to 4x4 matrices initially (up for discussion)

Adds tests for each matrix type

Tests are text template generated, therefore easy to add to each matrix type

Adds a .gitignore to the System.Numerics.Matrices directory to ignore TT temp files
{
return "Matrix4x4: "
+ $"{{|{M11:00}|{M21:00}|{M31:00}|{M41:00}|}}"
+ $"{{|{M12:00}|{M22:00}|{M32:00}|{M42:00}|}}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally did this with String.Format(...) and having the concat strings was "free" as they should have been concatenated at compile time. With interpolated strings, does it mean a bunch of strings will get alloc'd then concatenated on each ToString invocation?

@whoisj
Copy link
Contributor Author

whoisj commented Jul 8, 2015

/CC @KrzysztofCwalina how is the style/formatting. I can address those issues now and then worry about technical issues as they come up (they always come up). 😏

@KrzysztofCwalina
Copy link
Member

@whoisj, could you give me some context for this PR? Why is it being added to corfxlab? What is there some writeup describing the feature?

@JohanLarsson
Copy link

Mutable struct with equality feels scary. Why not immutable?

@whoisj
Copy link
Contributor Author

whoisj commented Jul 15, 2015

@KrzysztofCwalina it is here because you asked for it to be here (dotnet/corefx#1846).

There's no write up, though I suppose given enough time I could find the time to do a write up. I mostly added this because in several previous project we've had need for matrices and always ended up rolling our own. The .NET library doesn't supply any kind of matrices and there's not really a great reason not to.

@JohanLarsson I don't understand how this is scary? If you touch the struct you'll get a copy anyways, so any edits would be on your local copy - unless it was passed by ref in which case it is expected that you might edit it. All other structs are editable.

@KrzysztofCwalina
Copy link
Member

@whoisj, ah, thanks for pointing me to the thread. It's been so long. The thread will provide good context.

{
interface IMatrix
{
double this[int col, int row] { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why double? Would it make sense to make it IMatrix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my experience (mostly in the world of graphics) double is the most useful (though I could easily argue for float as well). Since there is no <T> where T is intrinsic or equivalent, I cannot make these generic. Therefore double was the closest thing as it solves the widest range of issues.

  • supports positive and negative values
  • supports a very wide range of values with decent precision (thanks to 64-bitness)
  • supports infinity
  • supports NaN
  • has built in CPU and GPU support
  • etc

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I realize double is a reasonable choice given how our type system and CLR work today. I would be interested in exploring what would we need to do in the type system and the CLR to enable scenarios like such generic matrix type.
We explored many solutions here in the past, but never pulled the trigger. Adding @CarolEidt who worked in this space int the past.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few good ideas in the Rosyln project bouncing around. My favorite is the ability to specify operate compliance on a type. Additionally, for optimizations there would be way to enforce that a type was not only a struct but a blittlable struct for optimized unsafe work on it.

Example:

public struct Matrix2x2<T>
    where T: struct, unsafe,
    operator +(T, T) => T,
    operator -(T, T) => T,
    operator *(T, T) => T,
    operator /(T, T) => T
{
    // ...
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, I will merge the PR. The goal of corefx lab projects IMO is to start with what's possible to day, force discussions on limitations, and then possibly try to fix solutions for these limitations.

KrzysztofCwalina added a commit that referenced this pull request Jul 16, 2015
@KrzysztofCwalina KrzysztofCwalina merged commit e3db25f into dotnet:master Jul 16, 2015
@whoisj whoisj deleted the system-numerics-matrices branch August 6, 2015 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants