rs-station / laue-dials

A package for analyzing Laue x-ray crystallography data using the DIALS framework.
https://rs-station.github.io/laue-dials/
MIT License
4 stars 3 forks source link

laue_dials.find_spots log does not contain meaningful information #33

Closed maggie-klureza closed 9 months ago

maggie-klureza commented 10 months ago

The log file written by laue_dials.find_spots (which seems to be laue_output.log rather than output.log - see other issue) contains a lot of text that is formatted like a spot finding log but doesn't actually contain useable information:

Finding spots in image 1 to 999...
Setting chunksize=31
Extracting strong spots from images
 Using multiprocessing with 8 parallel job(s)

Found %d strong pixels on image %d

Extracted %d spots
Removed %d spots with size < %d pixels
Removed %d spots with size > %d pixels
Calculated %d spot centroids
Calculated %d spot intensities
Filtered %d of %d spots by peak-centroid distance
Found %d strong pixels on image %d

Extracted %d spots
Removed %d spots with size < %d pixels
Removed %d spots with size > %d pixels
Calculated %d spot centroids
Calculated %d spot intensities
Filtered %d of %d spots by peak-centroid distance
Found %d strong pixels on image %d

Extracted %d spots
Removed %d spots with size < %d pixels
Removed %d spots with size > %d pixels
Calculated %d spot centroids
Calculated %d spot intensities
Filtered %d of %d spots by peak-centroid distance
Found %d strong pixels on image %d

This continues for many more iterations. It seems like the spot finding does proceed successfully, but it's hard to evaluate much from the log!

PrinceWalnut commented 10 months ago

The nature of this log works as intended, except that %d is not rendering properly here. That should be an integer number (that is the usual string format specifier in Python). I believe I've seen this issue when using multiprocessing and without multiprocessing it renders fine -- that suggests it's a DIALS-side issue. I'll confirm this and forward to the DIALS devs if so

maggie-klureza commented 10 months ago

Yes, the part where %d doesn't render is what I meant by not containing useful information - the fact that it's an integer number doesn't tell me much without knowing what that integer is. @dennisbrookner said he'd seen this as an issue with laue-dials previously (and it sounded like not in the context of non-laue DIALS?).

dennisbrookner commented 10 months ago

. @dennisbrookner said he'd seen this as an issue with laue-dials previously (and it sounded like not in the context of non-laue DIALS?).

Yes this is correct; I can confirm that I have seen the %d behavior in laue-dials and its "laue-index-assigner" predecessor. I don't recall ever seeing this in regular dials, but I don't know that I've really looked for it

PrinceWalnut commented 9 months ago

@maggie-klureza I was just looking at this on my end and I cannot reproduce it. It's possible this was a DIALS-side issue with multiprocessing and they've fixed it recently. Does this issue still occur for you?

maggie-klureza commented 9 months ago

I just made a new conda environment with fresh installations of dials and laue-dials ( conda install -c conda-forge dials, pip install laue-dials, as per the tutorial notebook) and reran the import and spotfinding in a new directory. I still get the same issue - spotfinding log is full of %d

PrinceWalnut commented 9 months ago

Which version of DIALS + laue-dials are you using? You can use pip list to get a complete set of packages with version numbers.

maggie-klureza commented 9 months ago

Looks like installing the packages as described above (aka as directed in the tutorial notebook) this morning led me to dials=3.17.0 and laue-dials=0.3. If that's an insufficiently recent version of dials, then the tutorial instructions on setting up a conda environment with the necessary packages should be revised.

PrinceWalnut commented 9 months ago

This issue is solved by this commit. DIALS has removed the force-2d parameter and the nonexistent parameter was causing this issue. laue-dials now has also removed the parameter.