-
Notifications
You must be signed in to change notification settings - Fork 5
chore: Update output data validations for frames and ctf #484
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
base: main
Are you sure you want to change the base?
Conversation
- update poetry lock
…ose low-to-high results in the same order" - add test for "Sum of exposureDose of all frames associated with a tilt series == totalFlux of tilt series" - add test for "max(acquisitionOrder) < number of frames - 1" - add test for "When isGainCorrected == False, a Gains entity exists for the run" - test_mdoc_frame_paths covers "frame file names match file names in mdoc section" - updated test_mdoc_frames to cover "number of mdoc sections == number of frame files on s3 == number of items in frames table/metadata" - There is no per_section_alignment_parameter.rotation_angle so I'm using tilt_angle. - test_tilt_axis_angle - add test for "number of frames >= # of per section parameters"
- -180 <= astigmaticAngle <= 180 - majorDefocus > minorDefocus - 0 <= phaseShift <= 2*pi - maxResolution > 0 - rawAngle matches mdoc angle as defined above - 0 <= zIndex <= (z-Dimension of tilt series - 1)
@uermel I don't see |
@uermel Should this be "max(acquisitionOrder) <= number of frames - 1." |
…ection_alignment_parameters
testing with the following datasets 10014, 10447, 10085, 10156 10014FAILED: "Tiltseries: sum of exposureDose of all frames associated with a tilt series == totalFlux of tilt series" Expected :50.0 |
@Bento007 Interesting! The validations already find mistakes! The total flux should indeed be 45 for this (based on the methods in the associated publication), I need to fix that in the config. Forgot to update total flux and only updated dose rate. Can we please compare this with a pytest.approx with a precision of ~1? Due to the precision of the values for dose rate this may never match exactly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates and expands the data validations for MDOC/Frames, Frames Metadata, and PerSectionParameter Metadata while adding additional pytest fixtures to support testing.
- Adds new tests for tiltseries and frame validations including exposure dose, per section parameters, tilt axis angle comparisons, etc.
- Updates error handling in tests to raise AssertionErrors instead of simple assertions.
- Enhances fixtures and helper functions to support new file filtering and metadata loading.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
ingestion_tools/scripts/data_validation/standardized/tests/test_tiltseries.py | Adds tests for exposure dose, per section parameter count, astigmatic angle, phase shift, max resolution, raw angle, and z_index validations. |
ingestion_tools/scripts/data_validation/standardized/tests/test_frame.py | Introduces tests for gain correction, acquisition order, and sorting of frames. |
ingestion_tools/scripts/data_validation/standardized/tests/test_alignments.py | Implements tests for tilt angle consistency between mdoc and alignment metadata with improved error handling. |
ingestion_tools/scripts/data_validation/standardized/fixtures/path.py | Updates file filtering to exclude .json files in addition to .mdoc. |
ingestion_tools/scripts/data_validation/standardized/fixtures/data.py | Adds a fixture to load frame metadata from frames_metadata.json. |
ingestion_tools/scripts/data_validation/shared/helper/tiltseries_helper.py | Updates tilt axis angle test to verify consistency with a 10° tolerance. |
ingestion_tools/scripts/data_validation/shared/helper/tilt_angles_helper.py | Adjusts error raising for raw tilt tests. |
ingestion_tools/scripts/data_validation/shared/helper/mdoc_helper.py | Enhances mdoc tests to include verification of frame metadata length and more robust error reporting. |
ingestion_tools/scripts/data_validation/shared/helper/angles_helper.py | Updates helper to utilize the passed angle_tolerance parameter. |
ingestion_tools/scripts/data_validation/standardized/tests/test_tiltseries.py
Outdated
Show resolved
Hide resolved
ingestion_tools/scripts/data_validation/standardized/tests/test_tiltseries.py
Outdated
Show resolved
Hide resolved
ingestion_tools/scripts/data_validation/standardized/tests/test_frame.py
Outdated
Show resolved
Hide resolved
ingestion_tools/scripts/data_validation/standardized/tests/test_alignments.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
A couple of the test datasets are failing but they look like actual errors with the data and not the tests |
if filesystem.exists(dst): | ||
return dst | ||
else: | ||
pytest.fail(f"Frames metadata file not found: {dst}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we update the message such that, it informs that Frames directory exists but the metadata.json doesn't?
pytest.skip("Alignment metadata missing per_section_alignment_parameters.") | ||
errors = [] | ||
for i, psap in enumerate(per_section_alignment_parameters): | ||
if (in_plane_rotation := psap.get("in_plane_rotation")) is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc, the in_plane_rotation is a 2x2 matrix. should there be a conversion for it before the following check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a specific way we do that conversion?
frame_metadata: Dict, | ||
gain_headers: Dict[str, Union[List[tifffile.TiffPage], MrcInterpreter]], # this is skipped if it is not found | ||
): | ||
if frame_metadata.get("is_gain_corrected") is False: # todo is none considered false? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would treat None
as False
for this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By that you mean the if
condition should be true, if is_gain_corrected
is None
, which would be equivalent to
if frame_metadata.get("is_gain_corrected") is False: # todo is none considered false? | |
if not frame_metadata.get("is_gain_corrected"): |
@allure.title("Frames: max(acquisitionOrder) <= number of frames -1") | ||
def test_max_acquisition_order(self, frame_metadata: Dict): | ||
acquisition_order_max = max(f.get("acquisition_order", 0) for f in frame_metadata["frames"]) | ||
assert acquisition_order_max <= len(frame_metadata["frames"]) -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: space after -
?
def test_exposure_dose(self, frame_metadata: Dict, tiltseries_metadata: Dict): | ||
assert sum(f.get("exposure_dose", 0) for f in frame_metadata["frames"]) == pytest.approx(tiltseries_metadata["total_flux"], abs=1) | ||
|
||
@allure.title("PerSectionParameters: number of frames >= # of per section parameters.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The label of what it is testing against like "PerSectionParameters" is very neat! 😄
ingestion_tools/scripts/data_validation/standardized/tests/test_tiltseries.py
Show resolved
Hide resolved
- fixture frames_meta_file failure message indicate the frames directory exists. - frame_metadata["is_gain_corrected"] is None is treated as - move mdoc_tilt_axis_angle outside TiltSeriesHelper to make it easier to share. - Add function to convert matrix to degress to be used in test_mdoc_tilt_axis_angle_in_alignment_per_section_alignment_parameters Signed-off-by: Bento007 <[email protected]>
Signed-off-by: Bento007 <[email protected]>
Description
resolves chanzuckerberg/cryoet-data-portal#1528
Validations for MDOC/Frames
Validations for Frames Metadata
Validations for PerSectionParameter Metadata
Extra
helper_angles_injection_errors
angle_tolerance
to work with more use cases.len(errors) == 0
assertion when multiple errors are caught. This reduces noise in the error message received.frame_metadata
andframes_meta_file
pytest fixtures to facilitate testing.