search

MasonProtter / Symbolics.jl

A symbolic math library written in Julia modelled off scmutils
MIT License
107 stars 15 forks source link

Define symbolic methods and their derivatives for more functions #9

Closed PerezHz closed 5 years ago

PerezHz commented 5 years ago

Fixes #6. This is currently work in progress. This PR defines derivatives using DiffRules instead of manual definitions. Started off by substituting current differentiation rules with their equivalencies in DiffRules, and locally tests pass. I think it'd be worthwhile to add more diff rules, as well as corresponding tests.

PerezHz commented 5 years ago

cc @MasonProtter

PerezHz commented 5 years ago

I really don't know why appveyor is giving us trouble again by not building this PR, but on my fork things are looking well

MasonProtter commented 5 years ago

Looks great! I’ll add a commit soon making a bunch more functions compatible with special functions so we can expand this list.

PerezHz commented 5 years ago

Thanks! Just added support for all Base functions, I think we can also handle special functions if we add SpecialFunctions as a dependency?

PerezHz commented 5 years ago

I went ahead and added also support for SpecialFunctions

PerezHz commented 5 years ago

Added diff rules for binary ops, although there are some method redefinition warnings, how would you suggest to handle those?

PerezHz commented 5 years ago

Got rid of the method redefinition warnings, and using DiffRules was able actually to define a bunch of special functions, together with their diff rule; although some (actually a lot of!) tests are not passing, I think it's possible to have most special functions defined together with their diff rules...

PerezHz commented 5 years ago

Thank you for fixing this! Can confirm that locally tests pass! Do you know what could be happening with appveyor?

MasonProtter commented 5 years ago

I don't know, but I can investigate soon.

I'm somewhat tempted to just turn off Appveyor and rely on knowing that the tests pass on mac and linux since we're not really doing anything that should be system dependant here. But I agree it'd be best to have it working.

PerezHz commented 5 years ago

Ok; I was thinking we maybe should also add the codecov badge to README.md?

PerezHz commented 5 years ago

Just added extra besselj and bessely methods which help make tests pass

MasonProtter commented 5 years ago

Okay, so I think I got it, but it's likely much more complicated than it needs to be...

MasonProtter commented 5 years ago

And I have no idea why Appveyor is no longer working...

But this now looks like it should work well. Think its time to merge?

PerezHz commented 5 years ago

Nice! Just looke at the travis log, and although it has green lights, there seem to be some method redefinitions? Can't reproduce those locally though... Otherwise, looks good to me!

PerezHz commented 5 years ago

About appveyor perhaps we could get rid of it for the time being, as you suggested...

MasonProtter commented 5 years ago

I just changed a comment to see if we get the method redefinition again, sometimes weird things happen. I also hoped it would somehow trigger Appveyor but no luck...

MasonProtter commented 5 years ago

Found the source of method redefinitions. It's strange that they didn't cause warnings on our local branches. I guess that's why CI is important.

It's even more bizarre that it didn't cause any errors in the tests...

MasonProtter commented 5 years ago

So at least when I merged it appveyor is running it. I think that's a good enough compromise. If Appveyor uncovers anything bad, we can always undo the merge and fix the problem.

MasonProtter commented 5 years ago

Do you know why Codecov doesn't know about the repo?

PerezHz commented 5 years ago

Do you know why Codecov doesn't know about the repo?

I really don't know what is happening with codecov, actually I've been having troubles logging into codecov the last few days; have double-checked that codecov is enabled for Symbolics.jl?