Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fixes, Runtime & Performance optimization #332

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

DrHazemAli
Copy link

This pull request includes significant optimizations and bug fixes.

ControlNetModel

  1. Constructor Improvements:
    Added type checks and exception handling.
    Simplified block creation with loops to reduce redundancy.
    Forward Method:

  2. Streamlined the forward pass to enhance performance and clarity.
    Ensured efficient tensor operations and minimized unnecessary computations.
    General Code Clean-up:

  3. Removed redundant checks and streamlined the logic.
    Organized methods and attributes for better readability and maintainability.

BPETokenizer
Initialization Improvements:

Enhanced error handling in the initializer init(mergesAt:vocabularyAt:).
Removed force unwrapping to prevent runtime crashes.
Tokenization Process:

Simplified the tokenization process to improve readability and efficiency.
Optimized the encode function to reduce redundant operations and improve performance.
Pair and Update Functions:

Utilized zip in pairs(for:) to create pairs more efficiently.
Refined update(_:merging:) to handle edge cases better and avoid unnecessary computations.
Helper Functions:

Introduced static helper functions for reading merges and vocabulary, including error handling.
These improvements ensure that the BPETokenizer class is more robust, efficient, and easier to maintain.
The changes address potential runtime errors and enhance the overall performance of the tokenizer.

Constructor (__init__ method):

Added type checks and proper exception handling.
Simplified repeated block creation using loops.
Forward Method:

Simplified the forward pass to ensure all operations are clear and efficient.
General Code Clean-up:

Removed redundant checks and code.
Ensured all methods and attributes are clear and logically ordered.
Initialization:

Added error handling for the init(mergesAt:vocabularyAt:) initializer.
Removed force unwrapping to avoid runtime crashes.
Tokenization:

Simplified the tokenization process and ensured that padding is handled efficiently.
Optimized the encode function for better performance by reducing redundant operations.
Pair and Update Functions:

Optimized pairs(for:) to use zip for creating pairs.
Improved update(_:merging:) to handle edge cases more effectively and avoid unnecessary computations.
Helper Functions:

Added static helper functions for reading merges and vocabulary with proper error handling.
Copy link

@kiruthikpurpose kiruthikpurpose left a comment

Choose a reason for hiding this comment

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

I like the way you contributed it. Seems like you removed all the comments and made it neat. Comments play an essential role for faster understanding without going their through each line of logic. Just add few comments to make sure to improve the readability. I appreciate your efforts!

@DrHazemAli
Copy link
Author

Alright

Added a check in the encode(word:) method to handle cases where no valid pairs exist.

Retained all comments from the original file to ensure readability and context.
Simplified ControlNetConditioningEmbedding block creation with list comprehension.

Reduced redundancy in down_blocks and controlnet_down_blocks initialization.

Retained all existing comments and added new ones to explain changes.
Copy link
Author

@DrHazemAli DrHazemAli left a comment

Choose a reason for hiding this comment

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

Comments restored.

@DrHazemAli
Copy link
Author

@kiruthikpurpose can you look into this?

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