adding some additional docstrings #81
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
skothenhill-nv
approved these changes
Aug 7, 2024
sub-packages/bionemo-example_model/src/bionemo/example_model/lightning_basic.py
Outdated
Show resolved
Hide resolved
jstjohn
reviewed
Aug 7, 2024
| value: The value tensor of shape [sk, b, ng, hn]. | ||
| attention_mask: The attention mask tensor of shape [b, np, sq, sk]. | ||
| attn_mask_type: The attention mask type, currently unused. Defaults to None. | ||
| packed_seq_params: The packed sequence parameters. Defaults to None. |
There was a problem hiding this comment.
packed_seq_params: The packed sequence parameters. These are used for context parallelism so will be needed to be implemented if we want to support this. Defaults to None.
jstjohn
approved these changes
Aug 7, 2024
jstjohn
reviewed
Aug 7, 2024
|
|
||
| !!! warning | ||
|
|
||
| Using F.gelu yields subtly wrong results. |
There was a problem hiding this comment.
TODO can we figure out why they're wrong and explain that?
There was a problem hiding this comment.
not necessarily in this PR just maybe track that we should improve this docstring.
There was a problem hiding this comment.
@farhadrgh -- tagging you here, do you have an explanation as to why?
Co-authored-by: Steven Kothen-Hill <[email protected]> Signed-off-by: Peter St. John <[email protected]>
|
/build-ci |
Comment on lines
+32
to
+46
| class BionemoModelConfig(Generic[Model], ABC): | ||
| """An abstract class for model configuration.""" | ||
|
|
||
| @abstractmethod | ||
| def configure_model(self, *args, **kwargs) -> Model: # D101 # noqa: D102 | ||
| def configure_model(self, *args, **kwargs) -> Model: | ||
| """Configures the model.""" | ||
| raise NotImplementedError() | ||
|
|
||
|
|
||
| class BionemoTrainableModelConfig(Generic[Model, Loss], BionemoModelConfig[Model]): # D101 # noqa: D101 | ||
| class BionemoTrainableModelConfig(Generic[Model, Loss], BionemoModelConfig[Model]): | ||
| """An abstract class for trainable model configuration.""" | ||
|
|
||
| @abstractmethod | ||
| def get_loss_reduction_class(self) -> Type[Loss]: # D101 # noqa: D102 | ||
| def get_loss_reduction_class(self) -> Type[Loss]: | ||
| """Returns the loss reduction class.""" |
There was a problem hiding this comment.
Is this ChatGPT output @pstjohn ?
There was a problem hiding this comment.
Yeah copilot most likely
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some additional work towards removing these docstring-ignore ruff rules