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

Hit indices #709

Merged
merged 5 commits into from
Apr 18, 2024
Merged

Hit indices #709

merged 5 commits into from
Apr 18, 2024

Conversation

dermen
Copy link

@dermen dermen commented Mar 14, 2024

I originally opened dials#198 , but the base repository was wrong!

So, in summary, this PR is for reprocessing XTC streams when you know the hit indices ahead of time. It works as expected when the psana datasource only includes a single run, but when doing things in multi-run mode, e.g. ds = psana.Datasource("exp=cxily7829:run=21-26:idx"), the indexing of events seems to get distorted. There are no "exceptions / code failures" from what I've seen, its just that the retrieved hits no longer correspond to what they were in the single run case. I honestly suspect its an issue with PSANA API not behaving as intended, most likely the bit where we get the event from the index, but it would be nice to trouble shoot for more eyes.

Long term, this could be useful if some preprocessing software or hardware determines the hit indices as data are rolling in from the detector.

@dermen dermen requested review from phyy-nx and dwpaley March 14, 2024 14:46
@dermen
Copy link
Author

dermen commented Mar 14, 2024

Im open to meetings if folks are interested

Copy link

codecov bot commented Mar 14, 2024

Codecov Report

Attention: Patch coverage is 10.00000% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 42.32%. Comparing base (78147cc) to head (e93cf4e).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #709      +/-   ##
==========================================
+ Coverage   42.15%   42.32%   +0.16%     
==========================================
  Files         188      188              
  Lines       16764    16892     +128     
  Branches     3196     3245      +49     
==========================================
+ Hits         7067     7149      +82     
- Misses       9052     9093      +41     
- Partials      645      650       +5     

@phyy-nx
Copy link
Contributor

phyy-nx commented Mar 19, 2024

Seems like a useful feature, thanks!

@dermen
Copy link
Author

dermen commented Mar 19, 2024

@phyy-nx , looking for a way to debug/ reproduce the multi-run failure . There are XTC streams in xfel regression, right ?

@phyy-nx
Copy link
Contributor

phyy-nx commented Mar 19, 2024

There are (for both Rayonix and CSPAD), but only single runs. If you want multiple runs you gotta go to a deposition.

Mmmm, actually you might be able to cheat by copying the xfel_regression runs and renaming the files. For example the Rayonix run is run 20 of experiment mfxo1916. If you copy the xtc streams to a fake run 21 it might be fine.

For reference, this script will put the right folders in your path for psana to use the xfel_regression data.

@jbeilstenedmands
Copy link

Is there a way we can make this more of a general feature in dxtbx/dials.import? I am also interested in this, and I don't think it should be limited to xtc stream data?

@jbeilstenedmands
Copy link

We discussed this a bit in today's dials meeting - I just wanted to clarify that my comment was just intended to generate discussion and show my interest in these kind of ideas. I do not wish to hold up this PR or ask you to make a general solution, that would be a lot of work! I will make a separate issue to discuss this more generally idea as I think it is really useful. Thanks!

@dermen
Copy link
Author

dermen commented Mar 22, 2024

@jbeilstenedmands , totally agree!

@ndevenish ndevenish merged commit 61b5a40 into main Apr 18, 2024
20 checks passed
@ndevenish ndevenish deleted the hit_indices branch April 18, 2024 14:58
toastisme pushed a commit to toastisme/dxtbx that referenced this pull request Jul 18, 2024
* uses hit indices for faster reprocessing
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