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

Add options for the propagation to the next register and to return the pure sum #14

Merged
merged 8 commits into from
Oct 30, 2018

Conversation

thepith
Copy link
Contributor

@thepith thepith commented Oct 29, 2018

Propagation of Data points

The method of propagating the values to the next register can affect the systematic error introduced by the multiple tau algorithm. Good examples are given in https://doi.org/10.1063/1.3491098 . Therefore there is a need to have a different method than averaging the data points when propagating to the next register. Using only one of the data points completely removes the systematic error at the cost of increasing the statistical error.

This is incorporated by adding the string option compress, where the following values are accepted:
"average" (default): average two measurements when pushing to the next
level of the correlator.
"first": use only the first value when pushing to the next level of the
correlator.
"second": use only the second value when pushing to the next level of
the correlator.

Return Pure Sum

When one wants to use a different normalization, than the one toggled by the normalize option, the problem arises, that one needs the values stored in normstat. Therefore a bool option was added, so that when toggled, the not normalized sum is returned along with the values of normstat. This enables any further normalization by the user.

Averaging the data can lead to large systematic errors
The value is given in the third column of the returned array
Give an option for the compression used, and print out normstat for the
cross-correlator
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.2%) to 93.814% when pulling a3a0d47 on thepith:master into e03b003 on FCS-analysis:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.2%) to 93.814% when pulling a3a0d47 on thepith:master into e03b003 on FCS-analysis:master.

@coveralls
Copy link

coveralls commented Oct 29, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling f6488fa on thepith:master into e03b003 on FCS-analysis:master.

@paulmueller
Copy link
Member

Thank you for this PR. Could you please add test functions such that the coverage remains at 100%?
Please also make sure that the documentation renders correctly, e.g. whether the options for compress are displayed correctly as a list or somesuch.

@thepith
Copy link
Contributor Author

thepith commented Oct 30, 2018

Hi,

i added tests and fixed the documentation.

Furthermore I noticed that when you abort the correlation you call

G = G[:idx - 1]
normstat = normstat[:idx - 1]
normnump = normnump[:idx - 1]

I think that this way you throw out the data at very long lag times. Is that intended? I believe it would be better to have

G = G[:idx]
normstat = normstat[:idx]
normnump = normnump[:idx]

@paulmueller
Copy link
Member

regarding your question: I think the "abort"-case is just a special case where there is actually no data in the last row of G. To me it just looks like bad programming. I was just not able to figure out a more elegant solution.

@paulmueller paulmueller merged commit 7b686e5 into FCS-analysis:master Oct 30, 2018
@paulmueller
Copy link
Member

Note: I renamed "return_sum" to "ret_sum", to be more consistent with numpy.

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.

None yet

3 participants