search

ravin-d-27 / PyDeepFlow

This is my own Deep Learning Package, optimized for performing Deep Learning Tasks and easy to learn and integrate into projects.
https://pypi.org/project/pydeepflow/
MIT License
18 stars 8 forks source link

Add L2 regularization and dropout options (#3) #23

Closed G0v1ndD3v closed 1 month ago

G0v1ndD3v commented 1 month ago

This is my work so far on implementing L2 regularization and dropout options. Yet to be tested. Feedback is welcome!

ravin-d-27 commented 1 month ago

Dear @G0v1ndD3v,

Thank you for your contribution! I appreciate your hard work, and overall, your implementation looks excellent. However, I did notice a few minor issues that I would like to bring to your attention for refinement:

  1. In model.py: It would be more in line with common conventions to change the reference to .regularization. This change will make the code more consistent with package standards.
    screenshot1

  2. In regularization.py: Similarly, it would be better to modify to .device for clarity and consistency.
    screenshot2

  3. I also attempted to run runner.py for testing, but I encountered some issues during execution. It would be helpful if you could review the script to ensure everything is functioning as expected.
    screenshot3

Once again, I appreciate your effort, and I look forward to your revisions. Please let me know if you need further clarification or assistance.

G0v1ndD3v commented 1 month ago

Updated the code to use .regularization in model.py and .device in regularization.py. Apologies for the oversight—I was used to working with an earlier version of the code that didn’t use relative imports and didn’t notice the switch.

Found my mistake in apply_l2_regularization. I was dividing weights by X_shape, which is a tuple (🤦), leading to a broadcasting error. Fixed it by dividing by X_shape[0]. I tested it in runner.py, and it's working fine now.

Thanks for pointing it out!

G0v1ndD3v commented 1 month ago

Keeping this PR as a draft for now; I still need to test with more datasets to cross-check. Will mark it ready once testing is complete.

ravin-d-27 commented 1 month ago

Cool @G0v1ndD3v, keep going !

ravin-d-27 commented 1 month ago

Hi @G0v1ndD3v,

I was facing this issue in your code. image

But, I found the issue. Just change this line 77 X.shape to X.shape[0] (File: model.py): image

To: image

And now it works: image

Just change this and make a PR. You did an amazing work !

G0v1ndD3v commented 1 month ago

Oh, I thought I had patched that in a previous commit! Line 77 looks different in our versions—maybe you forgot to pull the latest changes?

ravin-d-27 commented 1 month ago

Yes ! My bad. I didn't pull the latest changes. Thanks for letting me out ! I am merging this now ! Thank you @G0v1ndD3v !