JuliaDiff / ForwardDiff.jl

Forward Mode Automatic Differentiation for Julia
Other
892 stars 145 forks source link

Add a copy method for GradientConfig. #632

Closed tpapp closed 1 year ago

tpapp commented 1 year ago

This is motivated by https://github.com/tpapp/LogDensityProblemsAD.jl/pull/8, where the user can supply a GradientConfig, but the unified API for thread-safe invocation would require that the resulting object can be copyed.

Instead of relying on unexposed internals of this package, I thought it would be better to put this functionality here.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.05 :tada:

Comparison is base (1592fe9) 87.28% compared to head (c5dce04) 87.33%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #632 +/- ## ========================================== + Coverage 87.28% 87.33% +0.05% ========================================== Files 10 10 Lines 912 916 +4 ========================================== + Hits 796 800 +4 Misses 116 116 ``` | [Impacted Files](https://codecov.io/gh/JuliaDiff/ForwardDiff.jl/pull/632?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff) | Coverage Δ | | |---|---|---| | [src/config.jl](https://codecov.io/gh/JuliaDiff/ForwardDiff.jl/pull/632?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff#diff-c3JjL2NvbmZpZy5qbA==) | `88.13% <100.00%> (+0.86%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

KristofferC commented 1 year ago

Makes sense but shouldn't all configs have this method define then for consistency?

tpapp commented 1 year ago

I just realized that for ::AbstractConfig there is a copy that falls back to deepcopy, which should be sufficient. Closing.