Skip to content

Conversation

Jegp
Copy link
Collaborator

@Jegp Jegp commented Apr 9, 2025

Adds support for recursion in modules by

  1. Saving the output of a recursive module in the state
  2. Re-using the previous output from the state, defaulting to torch.zeros(1).

@Jegp Jegp added the enhancement New feature or request label Apr 9, 2025
@Jegp Jegp requested a review from pabogdan April 9, 2025 21:45
@Jegp Jegp self-assigned this Apr 9, 2025
@@ -105,20 +105,45 @@ def _construct_module_dict_recursive(
return owning_module



def _find_input_nodes(
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Optional] Standardising documentation to include descriptions of all arguments would be beneficial as more open-source collaborations come in. Even if these functions are internal, they should have a description of each of the params + a docstring.

Copy link
Collaborator

Choose a reason for hiding this comment

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

A standardised format everywhere would accelerate reading and understanding and probably help a bit for any LLM assisted (vibe) coding

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point! When you say standardizing documentation, do you have a specific format in mind? Or something that we should adhere to that we're not?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just meant choosing and sticking to a consistent format so that e.g. autodoc from sphinx knows how to format the parameters / arguments nicely when displaying them and doctests that can be run in ci/cd to ensure the example in the docs always pass

@Jegp
Copy link
Collaborator Author

Jegp commented Apr 11, 2025

There's still a bug with recurrent modules, hang on, I'll update the PR later....

@Jegp
Copy link
Collaborator Author

Jegp commented Apr 14, 2025

Done! This should cover even complex use cases with recursive loops >= 2 modules.

@Jegp Jegp force-pushed the feature-recursion branch from 725a41e to df139a2 Compare April 26, 2025 18:11
@Jegp Jegp merged commit 35623d8 into main Jun 16, 2025
15 checks passed
@Jegp Jegp deleted the feature-recursion branch June 16, 2025 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants