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

Fixing multiprocessing issue and minor bugfixes #108

Merged
merged 8 commits into from
Feb 13, 2024

Conversation

guillaumegrolleron
Copy link
Contributor

This PR is fixing the issue89 and improves the SPE fit validity flag as asked in issue106

Concerning the multiprocessing issue, I changed the the multiprocessing set up to create child processes from 'fork' to 'spawn', which is the default set up for mac and windows, and recommanded for linux.

I also improved the validity flag for the SPE fit (thanks to @tibaldo).

I also fixed some bugs found by @tibaldo in the photo_statistic module.

@guillaumegrolleron guillaumegrolleron changed the title Fixing multiprocessing issue and minor bugfix Fixing multiprocessing issue and minor bugfixes Feb 8, 2024
Copy link

codecov bot commented Feb 8, 2024

Codecov Report

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

Comparison is base (270c103) 30.55% compared to head (be6c27f) 30.72%.
Report is 13 commits behind head on master.

Files Patch % Lines
.../nectarchain/makers/component/spe/spe_algorithm.py 9.43% 96 Missing ⚠️
src/nectarchain/display/display.py 0.00% 17 Missing ⚠️
src/nectarchain/data/management.py 30.43% 16 Missing ⚠️
src/nectarchain/utils/logger.py 50.00% 9 Missing ⚠️
src/nectarchain/utils/io.py 52.94% 8 Missing ⚠️
...rchain/makers/calibration/gain/photostat_makers.py 33.33% 4 Missing ⚠️
src/nectarchain/makers/core.py 75.00% 2 Missing ⚠️
...chain/makers/component/photostatistic_component.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #108      +/-   ##
==========================================
+ Coverage   30.55%   30.72%   +0.16%     
==========================================
  Files          58       60       +2     
  Lines        3701     3811     +110     
==========================================
+ Hits         1131     1171      +40     
- Misses       2570     2640      +70     

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

Copy link
Member

@tibaldo tibaldo left a comment

Choose a reason for hiding this comment

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

Thank you for all these improvements!

@jlenain jlenain merged commit 9e5a232 into cta-observatory:master Feb 13, 2024
10 of 12 checks passed
@guillaumegrolleron guillaumegrolleron deleted the bug_multiproc_spe branch December 9, 2024 11:04
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.

SPE fit algorithm: account for fit convergence to set valid status Error SPE fitting with multiprocessing
3 participants