usnistgov / mass

Microcalorimeter pulse-analysis software
MIT License
6 stars 0 forks source link

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

Open joefowler opened 9 months ago

joefowler commented 9 months ago

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 commented 6 months ago

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


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

joefowler commented 6 months ago

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


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