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

Nullables #147

Merged
merged 4 commits into from
Jul 15, 2020
Merged

Nullables #147

merged 4 commits into from
Jul 15, 2020

Conversation

Happypig375
Copy link
Member

@Happypig375 Happypig375 commented Jul 12, 2020

and a lot of refactors to ensure null safety and simplify code

Also the 50th patch 🎉 on the way to 100 PRs!

Fixes #60

@WhiteBlackGoose
Copy link
Member

Thank you for your submission, I am still reviewing it though. I think we are going to merge it anyway, maybe some minor changes will be required. I have to check the "is" operator performance, which is surely slower, but we need to check how slower it is.

@WhiteBlackGoose
Copy link
Member

You cast here though

@WhiteBlackGoose
Copy link
Member

Despite the greater number of instructions...
http://code.re/y4VcQGwR

|   Method |     Mean |    Error |   StdDev |
|--------- |---------:|---------:|---------:|
|    CallM | 20.43 ns | 0.175 ns | 0.146 ns |
| CallMAlt | 17.62 ns | 0.269 ns | 0.225 ns |

@Happypig375
Copy link
Member Author

Here, I expanded the benchmark:

    public class IsBenchmark
    {
        public class A { public int Type { get; protected set; } }
        public class B : A { public B() => Type = 1; }
        public class C : A { public C() => Type = 2; }
        public class D : A { public D() => Type = 3; }
        public class E : A { public E() => Type = 4; }
        public class F : A { public F() => Type = 5; }
        readonly A[] Array;
        const int ArrayLength = 1000000;
        public IsBenchmark() {
            Array = new A[ArrayLength];
            var random = new Random(42);
            for (int i = 0; i < ArrayLength; i++)
                Array[i] =
                    random.Next(0, 5) switch
                    {
                        0 => new B(),
                        1 => new C(),
                        2 => new D(),
                        3 => new E(),
                        _ => new F(),
                    };
        }
        [Benchmark]
        public void IfType()
        {
            for (int i = 0; i < ArrayLength; i++)
                MIfType(Array[i]);
        }
        [Benchmark]
        public void SwitchType()
        {
            for (int i = 0; i < ArrayLength; i++)
                MSwitchType(Array[i]);
        }
        [Benchmark]
        public void Switch()
        {
            for (int i = 0; i < ArrayLength; i++)
                MSwitch(Array[i]);
        }
        [Benchmark]
        public void If()
        {
            for (int i = 0; i < ArrayLength; i++)
                MIf(Array[i]);
        }
        public string MIfType(A a)
        {
            if (a is B b)
                return M1(b);
            else if (a is C c)
                return M2(c);
            else if (a is D d)
                return M3(d);
            else if (a is E e)
                return M4(e);
            else if (a is F f)
                return M5(f);
            else throw null;
        }
        public string MSwitchType(A a)
        {
            return a switch
            {
                B b => M1(b),
                C c => M2(c),
                D d => M3(d),
                E e => M4(e),
                F f => M5(f),
                _ => throw null
            };
        }
        public string MSwitch(A a)
        {
            return a.Type switch
            {
                1 => M1((B)a),
                2 => M2((C)a),
                3 => M3((D)a),
                4 => M4((E)a),
                5 => M5((F)a),
                _ => throw null
            };
        }
        public string MIf(A a)
        {
            if (a.Type == 1)
                return M1((B)a);
            else if (a.Type == 2)
                return M2((C)a);
            else if (a.Type == 3)
                return M3((D)a);
            else if (a.Type == 4)
                return M4((E)a);
            else if (a.Type == 5)
                return M5((F)a);
            else throw null;
        }
        public string M1(B b) => b.ToString();
        public string M2(C c) => c.ToString();
        public string M3(D d) => d.ToString();
        public string M4(E e) => e.ToString();
        public string M5(F f) => f.ToString();
    }
|     Method |     Mean |    Error |   StdDev |
|----------- |---------:|---------:|---------:|
|     IfType | 20.41 ms | 0.045 ms | 0.040 ms |
| SwitchType | 21.56 ms | 0.041 ms | 0.038 ms |
|     Switch | 19.09 ms | 0.045 ms | 0.040 ms |
|         If | 17.67 ms | 0.044 ms | 0.041 ms |

Let's dump all switches and always use if! (lol)

@Happypig375
Copy link
Member Author

Happypig375 commented Jul 15, 2020

Add these two benchmarks:

        [Benchmark]
        public void SwitchStatementType()
        {
            for (int i = 0; i < ArrayLength; i++)
                MSwitchStatementType(Array[i]);
        }
        [Benchmark]
        public void SwitchStatement()
        {
            for (int i = 0; i < ArrayLength; i++)
                MSwitchStatement(Array[i]);
        }
        public string MSwitchStatementType(A a)
        {
            switch(a)
            {
                case B b: return M1(b);
                case C c: return M2(c);
                case D d: return M3(d);
                case E e: return M4(e);
                case F f: return M5(f);
                default: throw null;
            };
        }

        public string MSwitchStatement(A a)
        {
            switch (a.Type)
            {
                case 1: return M1((B)a);
                case 2: return M2((C)a);
                case 3: return M3((D)a);
                case 4: return M4((E)a);
                case 5: return M5((F)a);
                default: throw null;
            };
        }

And you have

|              Method |     Mean |    Error |   StdDev |
|-------------------- |---------:|---------:|---------:|
|              IfType | 20.58 ms | 0.044 ms | 0.041 ms |
|          SwitchType | 21.74 ms | 0.074 ms | 0.069 ms |
| SwitchStatementType | 19.72 ms | 0.037 ms | 0.033 ms |
|              Switch | 19.18 ms | 0.092 ms | 0.077 ms |
|     SwitchStatement | 19.07 ms | 0.043 ms | 0.036 ms |
|                  If | 17.27 ms | 0.081 ms | 0.071 ms |

@Happypig375
Copy link
Member Author

And notice the difference in both data. Even between two runs, there are 0.1mss of difference. Really shows that the 1ms~2ms difference is not that big of a deal.

@WhiteBlackGoose
Copy link
Member

Seems that you optimized the lib a lot, but it's unclear what exactly boosted it so much...

|          Method |            Your PR |                Old  |
|---------------- |-------------------:|--------------------:|
|       ParseEasy |        44,328.1 ns |        120,757.9 ns |
|       ParseHard |       178,760.4 ns |        372,742.6 ns |
|    SimplifyEasy |       397,973.5 ns |      1,277,858.5 ns |
|    SimplifyHard | 7,477,089,153.8 ns | 10,211,207,181.0 ns |
|        Derivate |        50,858.5 ns |        144,758.6 ns |
|       SolveEasy |     7,556,902.7 ns |     40,692,860.3 ns |
| SolveEasyMedium |       646,063.0 ns |      2,397,748.7 ns |
|     SolveMedium |   178,691,349.5 ns |    542,063,245.2 ns |
| SolveMediumHard | 2,754,157,261.1 ns | 10,421,226,739.4 ns |
|       SolveHard | 5,636,112,783.3 ns | 18,215,370,042.9 ns |
|        EvalEasy |       550,676.0 ns |      1,396,040.4 ns |
|     CompileEasy |        33,684.3 ns |         52,994.0 ns |
|     CompileHard |        73,738.5 ns |        124,980.2 ns |
|         RunEasy |           117.5 ns |            107.4 ns |
|       RunMedium |           844.0 ns |            786.0 ns |
|         RunHard |         1,427.9 ns |          1,347.9 ns |

@Happypig375
Copy link
Member Author

Happypig375 commented Jul 15, 2020

I remember that I got rid of some Downcasts since Downcasting is now automatic on creation.

Copy link
Member

@WhiteBlackGoose WhiteBlackGoose left a comment

Choose a reason for hiding this comment

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

Finished the review. I think it's better to add synonymic Create methods to Number (as it used to be), but I'll add them myself

&& this.Value != (EDecimal.Zero, EDecimal.One)
&& (this is RationalNumber || (this is ComplexNumber && !(this is RealNumber)))
? @$"\left({InternalLatexise()}\right)"
: InternalLatexise();
Copy link
Member

Choose a reason for hiding this comment

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

Here we should probably check the sign for the case, say, -3.54353663 which is real number

var num = RealNumber.Create(-3.4548234m);
var p = MathS.Pow(num, 5);
Console.WriteLine(p.Latexise());

{-3.4548234}^{5}

@WhiteBlackGoose WhiteBlackGoose merged commit 2081493 into asc-community:master Jul 15, 2020
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.

Use C# 8 nullable reference types
2 participants