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 calculation of average #723

Merged

Conversation

grzanka
Copy link
Contributor

@grzanka grzanka commented Mar 15, 2024

This PR introduces couple of changes in how multiple runs are averaged:

  • online version of mean and variance calculation methods are introduced and documented in averaging.py module
  • error attribute of page is now initialized to None, instead of array of same shape as data - this reduces memory if no error calculation is requested
  • tests are adjusted accordingly
  • support for python 3.8 is dropped to fully benefit from type hinting syntax introduced in 3.9

@grzanka grzanka linked an issue Mar 15, 2024 that may be closed by this pull request
@grzanka grzanka self-assigned this Mar 15, 2024
@grzanka grzanka marked this pull request as draft March 15, 2024 18:09
@grzanka grzanka requested a review from nbassler March 15, 2024 18:09
@grzanka grzanka marked this pull request as ready for review March 22, 2024 21:54
@grzanka
Copy link
Contributor Author

grzanka commented Mar 23, 2024

To test create a virtual environment and run:

pip install git+https://github.com/DataMedSci/pymchelper.git@722-check-if-weighted-average-from-multiple-runs-are-ok#egg=pymchelper

If working on HPC (Ares) remember to include new python ml python first, but do not load official pymchelper package via ml pymchelper.

@grzanka
Copy link
Contributor Author

grzanka commented Mar 23, 2024

TODO:

@grzanka
Copy link
Contributor Author

grzanka commented Mar 25, 2024

@nbassler that should be ready for review now

pymchelper/averaging.py Outdated Show resolved Hide resolved
pymchelper/averaging.py Show resolved Hide resolved
pymchelper/averaging.py Show resolved Hide resolved
pymchelper/averaging.py Show resolved Hide resolved
pymchelper/averaging.py Show resolved Hide resolved
Copy link
Member

@nbassler nbassler left a comment

Choose a reason for hiding this comment

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

maybe add the above comments for clarity?

(or maybe a table listing this incl stddev and stderr calculation)

Copy link
Member

@nbassler nbassler left a comment

Choose a reason for hiding this comment

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

Checked this against rev49 by sha256sum for timepix data . Only difference found was png images, which have changed due to the /prim.

OK

@grzanka grzanka merged commit 68576fc into master Mar 27, 2024
10 checks passed
@grzanka grzanka deleted the 722-check-if-weighted-average-from-multiple-runs-are-ok branch March 27, 2024 18:01
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.

check if weighted average from multiple runs are ok Add type hints Add type hints
2 participants