Skip to content

Remove or use dead code#39

Merged
BSchilperoort merged 11 commits intomasterfrom
19-unused-code
Oct 10, 2025
Merged

Remove or use dead code#39
BSchilperoort merged 11 commits intomasterfrom
19-unused-code

Conversation

@sverhoeven
Copy link
Collaborator

Fixes #19

@codacy-production
Copy link

codacy-production bot commented May 7, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+1.11% (target: -1.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (360e5d2) 2157 673 31.20%
Head commit (3104a82) 2083 (-74) 673 (+0) 32.31% (+1.11%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#39) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@BSchilperoort
Copy link
Collaborator

BSchilperoort commented Oct 10, 2025

I removed the rot_search.py script in 3104a82, because it has not worked on any release of powerfit. In 1.1.4 it didn't exist yet, and the file was included in 2.0.0, but the rotate_grid function did not exist in _powerfit.pyx at that release.

So it hasn't worked in >9 years.

@AnnaEngel98
Copy link
Contributor

I just discussed the other scripts with Alexandre. The rest can also be removed as we are not using it

@amjjbonvin
Copy link
Member

amjjbonvin commented Oct 10, 2025 via email

@BSchilperoort
Copy link
Collaborator

@AnnaEngel98 I also found out that the "rotate_grid3d" OpenCL kernel is unused, and contains a (known) bug;

// TODO fix why nan = 1 * 1 + 0 * 0, instead of 1
printf("c1 = c01 * dy1 + c11 * dy: %f = %f * %f + %f * %f\n", c1, c01, dy1, c11, dy);

The result of this operation are NaN values.

Instead, the rotate_image3d kernel is used to rotate the grid (which does lead to the same results on CPU and GPU). But this rotate_grid openCL kernel & python function remain in the code. Seeing as it's unused code that's not needed, shall I just remove this?

@AnnaEngel98
Copy link
Contributor

Yeah if it is unused and buggy it can definitely be removed

@codacy-production
Copy link

codacy-production bot commented Oct 10, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-5.18% (target: -1.00%) 33.33%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (360e5d2) 2157 673 31.20%
Head commit (f0e6bf4) 1991 (-166) 518 (-155) 26.02% (-5.18%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#39) 3 1 33.33%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@BSchilperoort BSchilperoort marked this pull request as ready for review October 10, 2025 11:52
@sverhoeven sverhoeven requested a review from Copilot October 10, 2025 11:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Removes significant amounts of dead/unused code across the powerfit_em package, including unused imports, commented-out code, and entire script files.

  • Removed unused imports across multiple files to clean up dependencies
  • Deleted commented-out code sections and debug print statements in test files
  • Removed entire script files and test modules that appear to be no longer used

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_powerfitter.py Removed unused imports and large sections of commented/skipped test code
tests/test_kernels.py Deleted entire test file (201 lines)
tests/test__powerfit.py Deleted entire test file (82 lines)
tests/run_tests.py Deleted entire test utility file (7 lines)
src/powerfit_em/scripts/rot_search.py Deleted entire script file (98 lines)
src/powerfit_em/scripts/fsc-curve.py Deleted entire script file (23 lines)
src/powerfit_em/scripts/atom2dens.py Deleted entire script file (20 lines)
src/powerfit_em/scripts/init.py Deleted entire script module (104 lines)
src/powerfit_em/report.py Removed unused import of FileType
src/powerfit_em/helpers.py Removed unused imports (os, errno, pi, volume module)
src/powerfit_em/correlators/kernels.cl Removed large rotate_grid3d kernel function (153 lines)
src/powerfit_em/correlators/gpu.py Removed unused import get_normalization_factor
src/powerfit_em/correlators/cpu.py Removed unused import get_normalization_factor
src/powerfit_em/correlators/clkernels.py Removed unused imports and rotate_grid3d method

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@BSchilperoort BSchilperoort merged commit b0971bb into master Oct 10, 2025
8 checks passed
@BSchilperoort BSchilperoort deleted the 19-unused-code branch October 10, 2025 11:57
sverhoeven added a commit that referenced this pull request Oct 15, 2025
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.

Remove dead code or use/document

5 participants