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

perf: improve performance of model & forward layer #616

Conversation

sibre28
Copy link
Contributor

@sibre28 sibre28 commented Apr 9, 2024

Closes #610

Summary of Changes

Fixed some bugs and improved the performance of some methods, there are still some changes to be made but it is helpful to merge this now as @Marsmaennchen221 and @Gerhardsa0 partly depend on it

@sibre28 sibre28 requested a review from a team as a code owner April 9, 2024 17:45
@sibre28 sibre28 linked an issue Apr 9, 2024 that may be closed by this pull request
@sibre28 sibre28 changed the title 610 improve fnn layer and model performance and remove some bugs feat: 610 improve fnn layer and model performance and remove some bugs Apr 9, 2024
Copy link
Contributor

github-actions bot commented Apr 9, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 11 0 0 1.08s
✅ PYTHON mypy 11 0 2.23s
✅ PYTHON ruff 11 0 0 0.23s
✅ REPOSITORY git_diff yes no 0.38s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

Copy link

codecov bot commented Apr 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (1ed2d56) to head (02e2996).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #616   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           61        62    +1     
  Lines         4609      4634   +25     
=========================================
+ Hits          4609      4634   +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lars-reimann lars-reimann changed the title feat: 610 improve fnn layer and model performance and remove some bugs feat: improve fnn layer and model performance and remove some bugs Apr 9, 2024
Copy link
Contributor

@WinPlay02 WinPlay02 left a comment

Choose a reason for hiding this comment

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

Just a friendly reminder (as I do not know about the planing in the Library group): Please add __hash__, __eq__ and __sizeof__ implementations where possible (as they are really valuable for running the library with the memoization infrastructure provided by the runner)

E.g. for FNNLayer, input_size and output_size could be used to construct a hash

Nothing of this needs to happen in this PR, this is just a reminder

@Gerhardsa0
Copy link
Contributor

Gerhardsa0 commented Apr 10, 2024

I just wanna start a discussion here, so mby we should start an extra discssion thread.

Should there be an abstract Layer class, which contains all signatures of the Layer class, so the User gets better Feedback on the function calls.
We cant let the FNNLayer stay everwhere, because a neural network does not only exist out of fnn layers.

Aswell I question myself, do we want the user to change the activation function by himself?, or do we keep only internal handle.
And why do we use the softmax function for outputlayers greater 2 and sigmoid else?

sibre28 and others added 6 commits April 14, 2024 16:21
…able_dataset' into 610-improve-fnn-layer-and-model-performance-and-remove-some-bugs

# Conflicts:
#	src/safeds/data/tabular/containers/_tagged_table.py
#	src/safeds/ml/nn/_model.py
…performance-and-remove-some-bugs' into 610-improve-fnn-layer-and-model-performance-and-remove-some-bugs
src/safeds/ml/nn/_model.py Outdated Show resolved Hide resolved
src/safeds/ml/nn/_model.py Outdated Show resolved Hide resolved
src/safeds/ml/nn/_model.py Outdated Show resolved Hide resolved
src/safeds/ml/nn/_model.py Outdated Show resolved Hide resolved
@Marsmaennchen221 Marsmaennchen221 changed the title feat: improve forward layer perf: improve performance of model & forward layer Apr 17, 2024
@Marsmaennchen221 Marsmaennchen221 merged commit e856cd5 into main Apr 17, 2024
9 checks passed
@Marsmaennchen221 Marsmaennchen221 deleted the 610-improve-fnn-layer-and-model-performance-and-remove-some-bugs branch April 17, 2024 13:22
lars-reimann pushed a commit that referenced this pull request Apr 17, 2024
## [0.21.0](v0.20.0...v0.21.0) (2024-04-17)

### Features

* add ARIMA model ([#577](#577)) ([8b9c7a9](8b9c7a9)), closes [#570](#570)
* Add ImageList class ([#534](#534)) ([3cb74a2](3cb74a2)), closes [#528](#528) [#599](#599) [#600](#600)
* more hash, sizeof and eq implementations ([#609](#609)) ([2bc0b0a](2bc0b0a))

### Performance Improvements

* Add special case to `Table.add_rows` to increase performance ([#608](#608)) ([ffb8304](ffb8304)), closes [#606](#606)
* improve performance of model & forward layer ([#616](#616)) ([e856cd5](e856cd5)), closes [#610](#610)
* lazily import our modules and external libraries ([#624](#624)) ([20fc313](20fc313))
* treat Tables specially when calling add_rows ([#606](#606)) ([e555b85](e555b85)), closes [#575](#575)
@lars-reimann
Copy link
Member

🎉 This PR is included in version 0.21.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@lars-reimann lars-reimann added the released Included in a release label Apr 17, 2024
@lars-reimann
Copy link
Member

@Safe-DS/library For future PRs, please separate renamings of public API elements from internal changes. One is a minor version bump (later even a major version bump once we have a 1.x.x release), the other is a patch version bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Included in a release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve FNN Layer and Model Performance
6 participants