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

Performance of (+,-,*,/)(::TaylorScalar,::Number): fresh version #81

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

lrnv
Copy link
Contributor

@lrnv lrnv commented Sep 25, 2024

Supplants #25 with an updated branch.

@tansongchen tansongchen merged commit f1f850b into JuliaDiff:main Sep 25, 2024
5 of 7 checks passed
@tansongchen
Copy link
Member

Looks good to me

The benchmarks failure is due to Zygote problem at v1.10 so I'm temporarily ignoring it

@lrnv
Copy link
Contributor Author

lrnv commented Sep 25, 2024

Thanks a lot for merging after all this time ! Maybe i'll reconsider moving to TaylorDiff :)

@tansongchen
Copy link
Member

@lrnv sorry for the long wait. I believe I need to explain what happened.

Previously, the PR wasn't merged because using tail etc confuses Zygote at type inference, and I am stuck with other obligations at school so haven't got enough time to find a solution.

However, recently I come back and will be working on this package for a while, and I plan to add chunk size as a feature (think of the Laplacian operator as an example). In order to do this, I must separate the primal (0-th order) value and tangents into two fields. Therefore, it becomes much natural to separate primal and tangents when doing binary operation with plain Numbers instead of promoting Numbers to TaylorScalar.

I will do this migration soon, and please let me know if you would like any further features to be added to TaylorDiff.jl!

@lrnv
Copy link
Contributor Author

lrnv commented Sep 26, 2024

@tansongchen Thanks a lot for the message, i am just really happy that you have time to work on it now !

The target for me was to ditch TaylorSeries.jl in both Copulas.jl and WilliamsonTransforms.jl, and I think I had an old PR to do so, I will revive it as soon as I have some time to get on it, and I might ask you more stuff in the process.

Please keep the good work on making the package as minimal as possible by moving stuff to extensions as soon as it makes sense -- I was and will still only use high order derivations, and dont really care about the rest. Will keep you informed when the Pr gets revived :)

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.

2 participants