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

Center of mass fix #179

Merged
merged 6 commits into from
Jun 6, 2024
Merged

Conversation

McHaillet
Copy link
Collaborator

@McHaillet McHaillet commented Jun 5, 2024

Fix for the center of mass. I am not fully sure about the unittest check yet though, maybe you have a better solution. Currently checking with the same method as I am using internally.

I also switched to box padding after recentering. That is the better order as the padding is anyway done with zeros.

Closes #173

The square seems to work nicely though. This is an example that was failing before:

centering_before_fix

And now works much better:

centering_after_fix

@McHaillet McHaillet requested a review from sroet June 5, 2024 14:58
Copy link

github-actions bot commented Jun 5, 2024

File Coverage Missing
All files 86%
src/pytom_tm/correlation.py 85% 94-95 126
src/pytom_tm/entry_points.py 78% 27 104-121 217-262 351-366 483-500 879-890
src/pytom_tm/extract.py 74% 110-121 179-186 212-215 303-321
src/pytom_tm/io.py 80% 16 26 35 44 53 62-66 75-79 83 87 95 98 144-146 199 233-234 333-336 361
src/pytom_tm/mask.py 80% 70 85 94-95
src/pytom_tm/parallel.py 89% 14-15 83 95-97 104 148
src/pytom_tm/plotting.py 16% 20-22 25-32 36-44 48-51 54-70 73-74 77-78 98-103 119-127 133-150 165-173 177-194 198-206 210-217 223 229 234 248-398
src/pytom_tm/tmjob.py 97% 276-277 289 295-297 432 472
src/pytom_tm/utils.py 78% 16-18
src/pytom_tm/weights.py 96% 61 292-293 300 492 495 498

Minimum allowed coverage is 85%

Generated by 🐒 cobertura-action against e624fc7

Copy link
Collaborator

@sroet sroet left a comment

Choose a reason for hiding this comment

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

One preference, feel free to ignore. LGTM otherwise

I am not fully sure about the unittest check yet though, maybe you have a better solution.

Current implementation looks good
One other solution might be to compare np.abs(generate_template_from_map(template, ...) and generate_template_from_map(np.abs(template), ...) which should be identical

@McHaillet
Copy link
Collaborator Author

Current implementation looks good One other solution might be to compare np.abs(generate_template_from_map(template, ...) and generate_template_from_map(np.abs(template), ...) which should be identical

I added this in. Will merge once the tests pass.

@McHaillet McHaillet merged commit c459f4d into SBC-Utrecht:main Jun 6, 2024
2 checks passed
@McHaillet McHaillet deleted the center_of_mass_fix branch June 6, 2024 10:28
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.

center of mass not working as expected
2 participants