-
Notifications
You must be signed in to change notification settings - Fork 2
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
Implement shifts and tilts as properties #15
Conversation
update version to dev version; add script to run tests with conditional coverage depending on presence of CUDA.
- store tilts/shifts in stack.tilts and stack.shifts properties - create Signal1D subclasses for TomoTilts and TomoShifts with shape validation and custom slicers - remove io.create_stack and implement in CommonStack constructor - override copy and deepcopy for CommonStack so tilts and shifts are copied - override save for CommonStack so tilts and shits are stashed in the metadata when writing to disk
- add more tests for tilts and shifts - get to 100% coverage - fix CommonStack signal constructor - better support for multiframe stacks - fix save_movie dimensions - fix plot_sinos due to new tilts property - make run_test.sh script remove existing coverage reports
make workflow choice of conda environment condtional
Tested Windows install via yml with conda on a non-CUDA machine. All tests are passing. Tested Linux install via yml with conda on a CUDA-capable machine. All tests are passing. |
- nvidia | ||
- defaults |
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.
Are the nvidia
and defaults
channels necessary here. In the past, defaults
and conda-forge
had binaries incompatibilities and I suspect that this is still the case. The cudatoolkit stack in conda-forge is in very good shape and works well, using conda-forge only would avoid any potential issue.
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.
This was originally copied from the astra instructions: https://astra-toolbox.com/docs/install.html#linux-windows-using-conda-for-python and since it worked, I did not change it. I'll play around with it in my upcoming changes and loosen it to just conda-forge
if that works.
Following up from our chat last week... This PR does a few things:
CommonStack
classTomoTilts
andTomoShifts
_additional_slicing_targets
functionality from HyperSpy to take care ofinav
andisig
slicing of these properties automaticallyhdf5
create_stack()
method in favor of using theTomoStack()
constructor directlycupy
dependency in the optional[gpu]
group (optional, since CuPy is not supported on MacOS)A note about astra and CIL... I tried to downgrade our astra dependency to 2.1.X to be compatible with CIL, which worked, but caused lots of other packaging grief because the earlier versions of astra are not on PyPI, so things would only work with conda in that case. It also required downgrading numpy to < 2.0. We can revisit if needed, but I'm hoping CIL may be able to update their dependency to the latest version without too much trouble.
Remaining todos after this PR:
Here is the test output on my CUDA system: