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

Improve diagnostics & reporting of lineup2() plan #1045

Merged
merged 12 commits into from
Dec 5, 2024

Conversation

prjemian
Copy link
Contributor

@prjemian prjemian added this to the 1.7.2 milestone Nov 26, 2024
@prjemian prjemian self-assigned this Nov 26, 2024
@jilavsky
Copy link

That looks great, thank you

@prjemian
Copy link
Contributor Author

success=True

Motor: 'motor'  Detector: 'noisy_det'
========== ====================
statistic  value               
========== ====================
n          19                  
centroid   -0.00604313560021479
x_at_max_y 0.07498907624123874 
fwhm       2.427762264595814   
variance   0.8626928293778106  
sigma      0.9288125910956475  
min_x      -3.0                
mean_x     0.9535573837387351  
max_x      4.90711476747747    
min_y      -0.08111386966031231
mean_y     0.29494471116616    
max_y      0.980899512651778   
success    True                
========== ====================

success=False

Motor: 'motor'  Detector: 'noisy_det'
========== ================================
statistic  value                          
========== ================================
n          19                             
centroid   -0.5391606368907506            
x_at_max_y 0.0                            
fwhm       3.0223874138499687             
variance   0.0                            
sigma      0.0                            
min_x      -3.0                           
mean_x     1.5                            
max_x      6.0                            
min_y      -0.09436187725503624           
mean_y     0.19753874119348702            
max_y      0.7724043891492777             
success    False                          
reasons    ['Computed peak sigma is zero.']
========== ================================

@prjemian
Copy link
Contributor Author

prjemian commented Nov 27, 2024

To get the peak analysis into the dictionary on a routine basis, need to insert a call for that analysis before the call to self.report():

self.analysis = array_statistics(
self._data[self._x_name],
self._data[self._detectors[0]],
)
if self.reporting:
self.report()

How to keep SignalStatsCallback generic? Move the peak analysis code into the callback class? Then refactor the success key into feature_found?

@prjemian
Copy link
Contributor Author

prjemian commented Nov 27, 2024

Each of these inner functions rely on content in SignalStatsCallback.analysis (a MMap structure). They could be moved into utils.statistics and called from a re-factored SignalStatsCallback. The unit tests would better target each of these these function's results.

def find_peak_position():

def strong_peak(stats) -> bool:

def too_wide(stats):

With that refactor, this decision would examine the appropriate SignalStatsCallback.analysis key:

if target is None:

@prjemian prjemian merged commit 7fcb7c4 into main Dec 5, 2024
10 checks passed
@prjemian prjemian deleted the 1044-lineup2-diagnostics branch December 5, 2024 00:31
@prjemian
Copy link
Contributor Author

prjemian commented Dec 5, 2024

@MDecarabas Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lineup2() plan needs better diagnostics, especially when it does not succeed
3 participants