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

Fluctuations at High Q values #70

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

Sasaank
Copy link

@Sasaank Sasaank commented Jan 26, 2019

This contains the method for computing the average curvature of a given signal. This will serve as a helper function for representing the fluctuations at high Q for IQ data (to be implemented in QOI.py).

This contains the method for computing the average curvature of a given signal. This is serve as a helper function for representing the fluctuations at high Q for IQ data (to be implemented in QOI.py).
This contains the method for computing the average curvature of a given
signal. This is serve as a helper function for representing the
fluctuations at high Q for IQ data (to be implemented in QOI.py).
@codecov-io
Copy link

codecov-io commented Jan 26, 2019

Codecov Report

Merging #70 into master will decrease coverage by 0.87%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
- Coverage   92.16%   91.29%   -0.88%     
==========================================
  Files          19       19              
  Lines         919      942      +23     
==========================================
+ Hits          847      860      +13     
- Misses         72       82      +10
Impacted Files Coverage Δ
xpdtools/tools.py 89.47% <100%> (+0.28%) ⬆️
xpdtools/tests/test_tools.py 100% <100%> (ø) ⬆️
xpdtools/pipelines/qoi.py 78.94% <33.33%> (-21.06%) ⬇️
xpdtools/calib.py 13.55% <0%> (-1.54%) ⬇️
xpdtools/pipelines/extra.py 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a2f622...7ca4c8d. Read the comment docs.

@CJ-Wright
Copy link
Member

This looks good, it needs a test

This contains the method for computing the average curvature of a given
signal. This is serve as a helper function for representing the
fluctuations at high Q for IQ data. The QOI file is updated in this commit.

A test for the avg_curvature function has also been added in this commit.
This contains the method for computing the average curvature of a given
signal. This is serve as a helper function for representing the
fluctuations at high Q for IQ data. The QOI file is updated in this commit.

A test for the avg_curvature function has also been added in this commit.
This contains the method for computing the average curvature of a given
signal. This is serve as a helper function for representing the
fluctuations at high Q for IQ data. The QOI file is updated in this commit.

A test for the avg_curvature function has also been added in this commit.
This contains the method for computing the average curvature of a given
signal. This is serve as a helper function for representing the
fluctuations at high Q for IQ data. The QOI file is updated in this commit.

A test for the avg_curvature function has also been added in this commit.
This contains the method for computing the average curvature of a given
signal. This is serve as a helper function for representing the
fluctuations at high Q for IQ data. The QOI file is updated in this commit.

A test for the avg_curvature function has also been added in this commit.
This contains the method for computing the average curvature of a given
signal. This is serve as a helper function for representing the
fluctuations at high Q for IQ data. The QOI file is updated in this commit.

A test for the avg_curvature function has also been added in this commit.
Copy link
Member

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

I am a little skeptical that this will do what we want it to, but let's try and see. Do you have some test data that samples liquid, some low-frequency oscillation and some high-frequency structural signal and make sure that this gives the expected answer?



def avg_curvature(y, x, high_val):
"""Computes the average of the curvature of a given signal past a
Copy link
Member

Choose a reason for hiding this comment

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

past -> beyond


def avg_curvature(y, x, high_val):
"""Computes the average of the curvature of a given signal past a
(currently) user provided high Q value, this will be used as the scalar
Copy link
Member

Choose a reason for hiding this comment

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

don't say what this will be used for. If it is just for that qoi it shouldn't be a separate function. If it has more broad application, it should be its own function but the docstring should just describe what it does.

This contains the method for computing the average curvature of a given
signal. This is serve as a helper function for representing the
fluctuations at high Q for IQ data. The QOI file is updated in this commit.

A test for the avg_curvature function has also been added in this commit.

Two methods for computing the pearsons correlation coefficient have been
added in this commit as well as tests for both.

Still need to implement them into the qoi file.
@@ -21,6 +21,13 @@ def max_gr_mean(pdf, **kwargs):
return locals()


def fluc_high_q(iq_comp, high_q_val=45, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

I think this need docstrings.

def fluc_high_q(iq_comp, high_q_val=45, **kwargs):
q = iq_comp.pluck(0)
iq = iq_comp.pluck(1)
high_q_fluc = avg_curvature(iq, q, high_q_val)
Copy link
Member

Choose a reason for hiding this comment

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

You might use the q and mean and combine them, see: https://github.com/xpdAcq/xpdtools/blob/master/xpdtools/pipelines/raw_pipeline.py#L340
This needs to run through a starmap.

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.

4 participants