feat: Add support for contrast limit #471
Conversation
| @@ -253,6 +262,10 @@ def get_output_volume_info(self) -> VolumeInfo: | |||
| output_prefix = self.get_output_path() | |||
| return get_volume_info(self.config.fs, f"{output_prefix}.mrc") | |||
|
|
|||
| def get_contrast_limits(self, method: Literal["gmm", "cdf"] = "gmm") -> tuple[float, float]: | |||
| output_prefix = self.get_output_path() | |||
| return get_volume_contrast_limits(self.config.fs, f"{output_prefix}.mrc", method=method) | |||
There was a problem hiding this comment.
Using the mrc instead of the zarr may possibly be problematic here due to memory issues
This computation was originally designed to run on the lowest resolution image from the zarr image pyramid
There was a problem hiding this comment.
If that's the case, then I think we perhaps have a problem, when reaching this point, at least in the tests, it seems that the .zarr file isn't produced. I'm not sure if that's comes from the mocks in the tests, or if that's a normal behavior (to not have the .zarr file produce).
ingestion_tools/pyproject.toml
Outdated
| @@ -53,7 +53,7 @@ imageio = "^2.33.1" | |||
| pytest = "^8.3.2" | |||
| boto3-stubs = {extras = ["s3"], version = "^1.34.34"} | |||
| mypy = "^1.8.0" | |||
| cryoet-data-portal-neuroglancer = { git = "https://github.com/chanzuckerberg/cryoet-data-portal-neuroglancer.git", tag = "v1.0.1" } | |||
| cryoet-data-portal-neuroglancer = { git = "https://github.com/chanzuckerberg/cryoet-data-portal-neuroglancer.git", tag = "v1.2.1"} | |||
There was a problem hiding this comment.
We can remove this now because the version is > 1.2.1
| t = time() | ||
| print("Start contrast limit computation for", tomogram) | ||
| contrast_limits = tomogram.get_contrast_limits() | ||
| print("Took", (time() - t), "s") |
There was a problem hiding this comment.
I think we could combine the print for the computed contrast limit and the time taken into one to reduce the number of prints
feat: faster contrast limit by not loading high res data
| @@ -174,7 +177,11 @@ def _create_config(self, alignment_metadata_path: str) -> dict[str, Any]: | |||
| volume_info = tomogram.get_output_volume_info() | |||
| voxel_size = round(volume_info.voxel_size, 3) | |||
| resolution = (voxel_size * 1e-10,) * 3 | |||
| layers = [self._to_tomogram_layer(tomogram, volume_info, resolution)] | |||
| t = time() | |||
| print("Start contrast limit computation for", tomogram) | |||
There was a problem hiding this comment.
Does this need to stay in the code? I assume it was here for performance tuning.
There was a problem hiding this comment.
@Bento007 Thanks for the feedback! Actually, we added some feedbacks as the contrast limit can take a little bit of time depending on the method, or the level of the pyramid that is used as input for the computation. The idea was to not have just the scripts that hangs for few seconds without any information about why it is. If that's something that you feel is not necessary, we can remove it.
There was a problem hiding this comment.
No, you can leave it. It would be worth adding a comment as to why we add this metric, since we aren't doing it anywhere else, but i'll leave that as optional.
Description
This PR links the contrast limit computation provided by cryoet-data-portal-neuroglancer
v1.2.1. The contrast limit is computed on the MRC file produced during the ingestion, and uses thegmmcomputation method by default.