Skip to content

Revise documentation for internal _copy utility#762

Merged
yebai merged 10 commits intomainfrom
yebai-patch-2
Sep 15, 2025
Merged

Revise documentation for internal _copy utility#762
yebai merged 10 commits intomainfrom
yebai-patch-2

Conversation

@yebai
Copy link
Copy Markdown
Member

@yebai yebai commented Sep 9, 2025

The PR cleans up docs on _copy and adds an overview of _copy's current behaviour for different types

Replacing and closes #760

Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 9, 2025

Mooncake.jl documentation for PR #762 is available at:
https://chalk-lab.github.io/Mooncake.jl/previews/PR762/

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>
@yebai yebai requested a review from Copilot September 9, 2025 20:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/utils.jl Outdated
Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 9, 2025

Performance Ratio:
Ratio of time to compute gradient and time to compute function.
Warning: results are very approximate! See here for more context.

┌────────────────────────────┬──────────┬──────────┬─────────────┬─────────┬────
│                      Label │   Primal │ Mooncake │ MooncakeFwd │  Zygote │ R ⋯
│                     String │   String │   String │      String │  String │   ⋯
├────────────────────────────┼──────────┼──────────┼─────────────┼─────────┼────
│                   sum_1000 │ 100.0 ns │      1.8 │         1.8 │     1.1 │   ⋯
│                  _sum_1000 │ 941.0 ns │     6.74 │        1.01 │  1480.0 │   ⋯
│               sum_sin_1000 │  6.53 μs │     2.22 │         1.4 │    1.71 │   ⋯
│              _sum_sin_1000 │   5.2 μs │     2.67 │        2.21 │   279.0 │   ⋯
│                   kron_sum │ 273.0 μs │     48.2 │         3.5 │     6.0 │   ⋯
│              kron_view_sum │ 325.0 μs │     42.6 │        3.38 │    11.4 │   ⋯
│      naive_map_sin_cos_exp │  2.12 μs │     2.16 │         1.4 │ missing │   ⋯
│            map_sin_cos_exp │  2.11 μs │     2.43 │        1.44 │    1.47 │   ⋯
│      broadcast_sin_cos_exp │  2.23 μs │     2.27 │        1.38 │    2.36 │   ⋯
│                 simple_mlp │ 198.0 μs │     6.47 │         3.1 │    1.71 │   ⋯
│                     gp_lml │ 241.0 μs │     8.82 │        2.08 │    3.76 │   ⋯
│ turing_broadcast_benchmark │  1.95 ms │     3.83 │        3.04 │ missing │   ⋯
│         large_single_block │ 390.0 ns │     4.42 │        1.98 │  4240.0 │   ⋯
└────────────────────────────┴──────────┴──────────┴─────────────┴─────────┴────
                                                               2 columns omitted

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@yebai yebai requested a review from gdalle September 10, 2025 16:13
@yebai
Copy link
Copy Markdown
Member Author

yebai commented Sep 10, 2025

@gdalle, I'd appreciate it if you could take a look at this.

Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@mhauru mhauru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a couple of proposals, but nothing show-stopping, seems fine to me.

Comment thread src/utils.jl Outdated
Comment thread src/utils.jl
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).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/utils.jl
Comment on lines +436 to +445
- `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
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Copy link
Copy Markdown
Member Author

@yebai yebai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mhauru -- I addressed your comments.

Signed-off-by: Hong Ge <3279477+yebai@users.noreply.github.com>
@yebai yebai merged commit b2863d3 into main Sep 15, 2025
89 checks passed
@yebai yebai deleted the yebai-patch-2 branch September 15, 2025 23:03
@yebai yebai mentioned this pull request Sep 15, 2025
yebai added a commit that referenced this pull request Mar 25, 2026
* 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>
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.

3 participants