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

FilterExperimental.compute mixes concerns which makes it hard to read, also doesn't work #260

Open
joefowler opened this issue Nov 29, 2023 · 2 comments
Labels
bug Something isn't working major
Milestone

Comments

@joefowler
Copy link
Member

Original report by Galen O'Neil (Bitbucket: oneilg, GitHub: oneilg).


The function is around 150 lines long. Most of the code has to do with bookkeeping what is needed to calculate each variant of filter. The code would be much clear if it had the the form

def compute_filter_from_parts(Rinv_sig, Rinv_orthogonal, other):
    dostuff()
    return filter_vec, variance, etc

def compute_filt_noconst(self):
    orthogonalities = [ts(unit)]
    get_Rinv(orthogonalities)
    filt_vec, variance, etc = compute_filter_from_parts(self.Rinv_sig, 
                                                        get_Rinv(orthogonalities), other)
    return filt_vec, variance, etc

def compute_filt_noexp(self):
    dosomething(tau)
    ...
    return filt_vec, variance, etc

also i get an error when running FilterExperimental.compute

@joefowler
Copy link
Member Author

Original comment by Joseph Fowler (Bitbucket: joe_fowler, ).


Did the recent PR at least fix the concern that it “also doesn’t work”?

@joefowler
Copy link
Member Author

Original comment by Galen O'Neil (Bitbucket: oneilg, GitHub: oneilg).


It at least run, and I believe the orthogonal to an exponential part works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working major
Projects
None yet
Development

No branches or pull requests

1 participant