Skip to content

Conversation

thecharlieblake
Copy link
Contributor

No description provided.

@thecharlieblake thecharlieblake merged commit 704edfa into main Apr 11, 2023
Copy link
Collaborator

@DouglasOrr DouglasOrr left a comment

Choose a reason for hiding this comment

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

Sorry, you didn't ask for a review, but there was one thing I wanted to mention (MLP), and another that I threw in because I couldn't stop...



@format_docstring(unary_constraint_docstring)
class MLP(nn.Module):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hesitant about calling this MLP if it only supports 2 layers (given the module is called modules not transformers).

self,
hidden_size: int,
act_fn: nn.Module = GELU(),
expansion_factor: int = 4,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Likewise hesitant r.e. MLP on these defaults. FFN?

f: Callable[..., Tensor],
output_scale: float,
grad_input_scale: float,
constraint: Optional[Callable[[float, float], float]] = gmean,
Copy link
Collaborator

Choose a reason for hiding this comment

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

(Low) perhaps worth defining Constraint = Callable[[float, float], float] somewhere, as Optional[Constraint] might read better(?)

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