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

Don't set batch offset when creating image sequences #662

Closed
wants to merge 2 commits into from

Conversation

jbeilstenedmands
Copy link

This should now be unnecessary following #634 and related changes.

Removes an unintended use of the batch offset property

Fixes dials/dials#1347

Will not be merged without thorough testing and discussion.

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #662 (cb9ba90) into main (cd03663) will decrease coverage by 0.01%.
Report is 1 commits behind head on main.
The diff coverage is 80.00%.

❗ Current head cb9ba90 differs from pull request most recent head 06657c4. Consider uploading reports for the commit 06657c4 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #662      +/-   ##
==========================================
- Coverage   41.97%   41.96%   -0.01%     
==========================================
  Files         182      182              
  Lines       15885    15886       +1     
  Branches     3047     3047              
==========================================
  Hits         6667     6667              
- Misses       8608     8609       +1     
  Partials      610      610              

@ndevenish
Copy link
Collaborator

Holding off for more testing

@dwpaley
Copy link

dwpaley commented Dec 14, 2023

Running XFEL tests here:
https://github.com/cctbx/cctbx_project/tree/test_dxtbx_662

@dwpaley
Copy link

dwpaley commented Jan 16, 2024

Currently all good with respect to XFEL CI tests :)

@jbeilstenedmands
Copy link
Author

I have run the dials and xia2 tests, however some of the xia2.ssx tests are failing with this change, due to the use of the batch offset in dials.ssx_integrate when slicing an imageset.
I need to investigate further whether I can work around this, I thinkn in theory it should be possible.

@jbeilstenedmands
Copy link
Author

After further testing, I believe we do still need this functionality for recording when the first image in an imageset is not 0/1, even if it is not used for slicing the imageset, so I am closing this. Any later uses of the batch offset, e.g. in dials.export, should take this into account.

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.

batch_offset causes failure in export
4 participants