Skip to content

Update pipelines to match changes to image saver and rescaler methods #457

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

Merged
merged 6 commits into from
Oct 2, 2024

Conversation

yousefmoazzam
Copy link
Collaborator

@yousefmoazzam yousefmoazzam commented Sep 26, 2024

These changes are needed on the httomo side to accommodate when the following changes are released in httomolib and httomolibgpu (ie, when these changes make their way into the conda packages that httomo installs during CI):

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have made corresponding changes to the documentation

@yousefmoazzam
Copy link
Collaborator Author

The two failing tests are both related to how the image saver wrapper is auto-inserted after the method that has the sweep:

test_pipeline.py::test_run_gpu_pipeline_sweep_cor
test_pipeline.py::test_run_gpu_pipeline_sweep_paganin

The tests fail because the image saver method has been auto-inserted, but the input data has not been rescaled (the image saver no longer supports rescaling the input). Thus, it would seems like, for sweeps, the rescale_to_int method needs to be auto-inserted along with the save_to_images method to fix this issue.

The `save_to_images` method no longer provides rescaling of the input
data. This means that auto-insertion of the image saver method also
requires auto-insertion of the rescaler method before the image saver,
in order to rescale the data before it's passed to the image saver.
@yousefmoazzam yousefmoazzam marked this pull request as ready for review October 2, 2024 09:12
@dkazanc
Copy link
Collaborator

dkazanc commented Oct 2, 2024

Looks all fine, thanks for that! Slightly disappointing that we need to auto-insert always with perc_range_min: 0.0 and perc_range_max: 100.0, but at the moment there is not much we can do about it. An alternative can be to incorporate a possibility of saving 32-bit floating point tiffs (we will wait if this is needed).

I hope that in future when we have a percentile based scaling our auto range can be on a safer side by defaulting to say [5,95] range or something. This should help to avoid dragging some outliers into the rescaled data.

@yousefmoazzam yousefmoazzam merged commit 6af2e2e into main Oct 2, 2024
6 of 7 checks passed
@yousefmoazzam yousefmoazzam deleted the update-pipelines-image-saver-and-rescaler branch October 2, 2024 13:20
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.

2 participants