rs-station / reciprocalspaceship

Tools for exploring reciprocal space
https://rs-station.github.io/reciprocalspaceship/
MIT License
28 stars 12 forks source link

Faster parser for CrystFEL stream files #216

Closed kmdalton closed 3 months ago

kmdalton commented 1 year ago

I wrote a faster version of the stream file parser. It mostly just uses more numpy and less pandas while vectorizing operations within a crystal block.

codecov-commenter commented 1 year ago

Codecov Report

Merging #216 (d0ee0d6) into main (859e75c) will decrease coverage by 1.81%. The diff coverage is 74.39%.

@@            Coverage Diff             @@
##             main     #216      +/-   ##
==========================================
- Coverage   98.39%   96.58%   -1.81%     
==========================================
  Files          45       45              
  Lines        1803     1845      +42     
==========================================
+ Hits         1774     1782       +8     
- Misses         29       63      +34     
Flag Coverage Δ
unittests 96.58% <74.39%> (-1.81%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
reciprocalspaceship/io/crystfel.py 75.86% <74.39%> (-18.84%) :arrow_down:

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

DHekstra commented 1 year ago

Does this/could this also address https://github.com/rs-station/reciprocalspaceship/issues/213 ?

kmdalton commented 1 year ago

@DHekstra , i don't know if the information you want is available in the stream file. i would need to see an example.

JBGreisman commented 1 year ago

This looks pretty good to me, but I'll do another look through before approving/merging.

kmdalton commented 1 year ago

There are a couple of issues that still need to be addressed 1) Doeke provided me some more recent stream files, and they seem to break some of my formatting assumptions. 2) The current version doesn't respect that some files have the photon energy measured per frame. My code just uses the photon energy in the header for every crystal. This is a problem for XFEL data where the peak energy changes from shot to shot.

I'm going to hold off until I have a bit more time to look through it.

DHekstra commented 1 year ago

@kmdalton, what is the status of this? I'm revisiting some stream files next week, and could test drive your parser.

kmdalton commented 1 year ago

I was not able to parse some stream files @DHekstra gave me. There are some formatting differences between stream files that I haven't accounted for. I'm unlikely to have time to finish this soon.

DHekstra commented 1 year ago

that's alright. I have a slow but functional parser.

JBGreisman commented 5 months ago

Just checking in here -- is there anything left to do on this PR or should it be merged into main?

kmdalton commented 5 months ago

I discovered this version in was incompatible with some stream files provided by @DHekstra. I haven't been able to diagnose the issue yet. I would recommend we hold off on merging.

JBGreisman commented 3 months ago

Does this need to be merged anymore? Or will this be superseded by #260 ?

kmdalton commented 3 months ago

superseded by #260