mathnet / mathnet-symbolics

Math.NET Symbolics
http://symbolics.mathdotnet.com
MIT License
341 stars 66 forks source link

Add simple rules to newly added functions #49

Closed diluculo closed 6 years ago

diluculo commented 6 years ago

Following rules are added to newly added functions. Please see #44.

(2) undefined and infinities : sin(infinity) = undefined (3) zero, one, pi, and j: sin(pi) = 0, sin(j) = j*sinh(1) (7) negative argument: sin(-x) = -sin(x) (9) forward-inverse identities: sin(asin(x)) = x

This is including the PR #48.

FoggyFinder commented 6 years ago

This is including the PR #48.

So, maybe #48 should be closed?

diluculo commented 6 years ago

@FoggyFinder The first commit(Fix derivative..., #48) is a bug fix and is required. However, the second commit(Add rules...) can be "optional". I think the latter is simple enough and appropriate in the scope of Symbolics, but it made the operator module very very long and dirty. I don't know how it affects the performance of the F# code. Here, "optional" means that there will be a better workaround structurally.

Is it possible to split the very long Operators module into small ones? i.e. elementary, trigonometric, hyperbolic, gamma, error, and Bessel functions.

diluculo commented 6 years ago

To resolve conflicts, I removed changes to UnitTests in the my last commit, rebased to upstream/master, and update UnitTests. Now, can I push the local branch? Otherwise, is it better to close this and send new PR?

FoggyFinder commented 6 years ago

To resolve conflicts, I removed changes to UnitTests in the my last commit, rebased to upstream/master, and update UnitTests. Now, can I push the local branch? Otherwise, is it better to close this and send new PR?

I think the first option (update this PR) easier - you can just sent new commits to your fork, can you?

diluculo commented 6 years ago

@FoggyFinder Thanks, but I fail to push...

Error encountered while pushing to the remote repository: rejected Updates were rejected because the tip of your current branch is behind its remote counterpart. Integrate the remote changes before pushing again.

It may be related already merged commit (#48).

FoggyFinder commented 6 years ago

Dunno, maybe you did something wrong. I'm not very good with git, I'd ask someone in chat. Maybe in C# discord chat or room on StackOverFlow

cdrnet commented 6 years ago

I can take care of resolving the conflict if needed (seems to be straight forward to fix). Sorry for messing the PR up with my changes.

diluculo commented 6 years ago

Actually, simple solution may be closing this and sending new PR with revised branch. I am just wondering if I can do it?

cdrnet commented 6 years ago

I think you could even force push your changes to the same branch AddSimpleRules and keep using this PR for it.

diluculo commented 6 years ago

@cdrnet Many thanks. I didn't know git push --force

cdrnet commented 6 years ago

Thanks!