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.

Sign up for GitHub

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

Jump to bottom

Initial QOIs #61

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

Initial QOIs #61

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
930f7a7
Initial QOIs
Sasaank Nov 7, 2018
89d0115
Initial QOIs
Sasaank Nov 30, 2018
794fa54
Initial QOIs-fixed docstring issues
Sasaank Nov 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions xpdtools/pipelines/qoi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,3 @@

pdf_argrelmax_kwargs = pdf_peaks.upstreams[0].kwargs
mean_argrelmax_kwargs = mean_peaks.upstreams[0].kwargs







65 changes: 35 additions & 30 deletions xpdtools/tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,13 +585,13 @@ def inner(x, *args, **kwargs):
return inner


def max_value(g, r):
def max_value(g):
"""Returns largest value
Copy link
Member

Choose a reason for hiding this comment

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

This is not so helpful. Bear in mind what the docstring is for. It is for a user to understand what this function is for and how it should be used. Also, the docstring shouldn't include the function name, which is effectively what your's does.

In other words, why do we (or rather a user) need this function (rather than just typing np.amax())?


Parameters
----------
g : ndarray g(r) of the pdf
r : ndarray of corresponding r values
g : ndarray
g(r) of the pdf

Returns
-------
Expand All @@ -602,13 +602,16 @@ def max_value(g, r):
return np.amax(g)


def tallest_peak(g,r):
def tallest_peak(g, r):
"""Returns r,g(r) of the tallest peak

Parameters
----------
g : ndarray g(r) of the pdf
r : ndarray of corresponding r values
g : ndarray
g(r) of the pdf

r : ndarray
corresponding r values

Returns
-------
Expand All @@ -617,7 +620,7 @@ def tallest_peak(g,r):
float :
g(r) value of the tallest peak

"""
"""
peaks = sig.find_peaks(g)
height = []
r_val = []
Expand All @@ -627,41 +630,43 @@ def tallest_peak(g,r):
return r_val[np.argmax(height)], np.amax(height)


def total_counts(g,r):
def total_counts(g):
Copy link
Member

Choose a reason for hiding this comment

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

I'm not certain we need this function if it is only calling len on an array

"""Returns total number of data points in graph

Parameters
----------
g : ndarray g(r) of the pdf
r : ndarray of corresponding r values

Returns
-------
float :
total number of data points
Parameters
----------
g : ndarray
g(r) of the pdf

Returns
-------
int :
total number of data points

"""
"""
return len(g)


def average_pearson(group, g,r):
def average_pearson(group, g, r):
Copy link
Member

Choose a reason for hiding this comment

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

Can we also compute the complete pearson correction? (Maybe as a separate function?)

Copy link
Author

Choose a reason for hiding this comment

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

I think I may be confused on the stats here, to my knowledge the pearson's coefficients can be used on two different datasets. Is there a separate coefficient to compare a dataset to an entire group of datasets.

Copy link
Member

Choose a reason for hiding this comment

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

You could return the list of pearson's coefficents for the new pdf and all the prior pdfs

Copy link
Member

Choose a reason for hiding this comment

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

@Sasaank poke

Copy link
Author

Choose a reason for hiding this comment

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

pushed this into the other pr

Copy link
Member

Choose a reason for hiding this comment

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

ah ok

"""Computes the average pearson of this PDF with the rest of the group

Parameters
----------
g : ndarray g(r) of the pdf
r : ndarray of corresponding r values
group : ndarray of PDFs (PDFs as tuples (g,r))
Returns
-------
float :
Average pearson's coefficient
Parameters
----------
g : ndarray
g(r) of the pdf

r : ndarray
corresponding r values
group : ndarray
Group of PDFs (PDFs as tuples (g,r))
Returns
-------
float :
Average pearson's coefficient

"""
"""
val = 0
for i in group:
r,p = stats.pearsonr((g, r), i)
r, p = stats.pearsonr((g, r), i)
val = val + r
return val/len(group)