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

🧹 Remove redundant backsweep flag from GaussianActivationParameters #1423

Merged
merged 1 commit into from
Jan 27, 2024

Conversation

s-weigand
Copy link
Member

Just a small additional clean-up after #1422 to get rid of the backsweep boolean flag, which is a residual of the < 0.8 model syntax when we had this as user input.

Change summary

Checklist

  • ✔️ Passing the tests (mandatory for all PR's)

@s-weigand s-weigand requested review from jsnel, joernweissenborn and a team as code owners January 27, 2024 18:22
Copy link
Contributor

Binder 👈 Launch a binder notebook on branch s-weigand/pyglotaran/remove-redundant-backsweep-flag

Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

PR Type: Refactoring

PR Summary: The pull request removes the 'backsweep' boolean flag from various components related to Gaussian activation in the codebase. This change simplifies the parameter model by eliminating the need for a flag that was previously used as user input in older versions of the system.

Decision: Comment

📝 Type: 'Refactoring' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
✅ Small diff: the diff is small enough to approve with confidence.
No details provided.

General suggestions:

  • Ensure that the removal of the 'backsweep' flag and the associated changes do not introduce any division by zero errors, especially when 'backsweep_period' is zero.
  • Verify that the removal of the 'type:ignore[operator]' comment does not lead to any unresolved type issues or introduce new ones.
  • Review all parts of the codebase that interact with the modified functions and classes to confirm that they are compatible with the updated function signatures and class attributes.
  • Consider the broader impact of these changes on the system's behavior, particularly in cases where the 'backsweep' condition was previously used to guard against certain calculations.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

glotaran/builtin/elements/kinetic/matrix.py Show resolved Hide resolved
Copy link

codecov bot commented Jan 27, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (5f16090) 84.9% compared to head (438a7e9) 84.9%.

Files Patch % Lines
glotaran/builtin/elements/kinetic/matrix.py 0.0% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           staging   #1423     +/-   ##
=========================================
- Coverage     84.9%   84.9%   -0.1%     
=========================================
  Files           91      91             
  Lines         3746    3744      -2     
  Branches       728     728             
=========================================
- Hits          3182    3180      -2     
  Misses         450     450             
  Partials       114     114             

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

Copy link
Member

@jsnel jsnel left a comment

Choose a reason for hiding this comment

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

Lgtm

@jsnel jsnel merged commit 491a7b6 into glotaran:staging Jan 27, 2024
21 of 34 checks passed
@jsnel jsnel deleted the remove-redundant-backsweep-flag branch January 27, 2024 20:56
jsnel pushed a commit to jsnel/pyglotaran that referenced this pull request Jun 2, 2024
…lotaran#1423)

Just a small additional clean-up after glotaran#1422 to get rid of the backsweep boolean flag, which is a residual of the < 0.8 model syntax when we had this as user input.
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.

3 participants