Revise documentation for internal _copy utility#762
Conversation
Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
|
Mooncake.jl documentation for PR #762 is available at: |
Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
Expanded documentation for the `_copy` function to include examples of current behavior for various types. Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR updates the documentation for the internal _copy utility function, improving clarity and readability. The changes replace verbose technical documentation with a more concise format that focuses on practical examples of current behavior.
- Replaces detailed implementation requirements with warning about internal API status
- Converts extensive code examples to a concise list of type-specific behaviors
- Updates comments in dual.jl and codual.jl to remove redundant "immutable" references
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/utils.jl | Simplifies _copy documentation with warning block and behavior examples |
| src/dual.jl | Updates comment to remove redundant "immutable" reference |
| src/codual.jl | Updates comment to remove redundant "immutable" reference |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
|
Performance Ratio: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@gdalle, I'd appreciate it if you could take a look at this. |
Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
mhauru
left a comment
There was a problem hiding this comment.
I have a couple of proposals, but nothing show-stopping, seems fine to me.
| Internal protocol for creating copies of AD-related data structures. This function is used | ||
| throughout the automatic differentiation system to create appropriate copies of rules, | ||
| caches, and other internal data structures when building new AD contexts. | ||
| Utility for copying AD-related data structures (e.g. rules, caches, and other internals). |
There was a problem hiding this comment.
Would it be useful to say something about where _copy gets used? Or something else that would help answer the question of, if I was to implement my own type Foo, what should I make _copy(::Foo) do?
There was a problem hiding this comment.
It is a bit awkward to say any more about that for now -- the _copy function is a bit scattered and thus requires some tidy up down the road.
| - `CoDual`, `Dual` types → no copying needed | ||
| - `Stack` types → create a new empty instance | ||
| - Composite types (e.g. `Tuple`, `NamedTuple`) → recursively copy elements | ||
| - Misty closure → construct a new Misty closure with deep copy of captured data | ||
| - Tangent types (`PossiblyUninitTangent`) → copy conditionally based on initialisation state | ||
| - Forward/reverse data types (e.g. `FData`, `RData`, `LazyZeroRData`) → recursively copy wrapped data | ||
| - `RRuleZeroWrapper` → recursively copy the wrapped rule into a new instance | ||
| - `DerivedRule` → construct new instances with copied captures and caches | ||
| - `LazyFRule`, `LazyDerivedRule` → construct new lazy rules with the same method instance and debug mode | ||
| - `DynamicFRule`, `DynamicDerivedRule` → construct new dynamic rules with an empty cache and the same debug mode |
There was a problem hiding this comment.
Might it make sense to document these in the same place where they are implemented? Some of these implementations must live in other files, and could be added as docstrings to the methods that implement them. Less risk of the documentation falling out of sync with the implementation.
There was a problem hiding this comment.
That is already done as comments, the point of repeating them here is to have an overview.
Co-authored-by: Markus Hauru <mhauru@turing.ac.uk> Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
* Revise documentation for internal `_copy` utility Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com> * Fix comment formatting in codual.jl Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com> * Fix comment formatting in dual.jl Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com> * Tighten `_copy` docs Expanded documentation for the `_copy` function to include examples of current behavior for various types. Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com> * Enhance copy rules for various data types Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com> * Fix formatting and update comments in utils.jl Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com> * Update utils.jl Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com> * Update src/utils.jl Co-authored-by: Markus Hauru <mhauru@turing.ac.uk> Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com> * Fix syntax in debug_mode documentation example Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com> --------- Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com> Co-authored-by: Markus Hauru <mhauru@turing.ac.uk>
The PR cleans up docs on
_copyand adds an overview of_copy's current behaviour for different typesReplacing and closes #760