Codecov Report

Attention: Patch coverage is 79.71831% with 72 lines in your changes missing coverage. Please review.

Project coverage is 80.77%. Comparing base (b0fd882) to head (872bc60).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##             main     #474      +/-   ##
==========================================
+ Coverage   79.85%   80.77%   +0.91%     
==========================================
  Files         187      189       +2     
  Lines       16822    17226     +404     
  Branches     2213     2282      +69     
==========================================
+ Hits        13434    13914     +480     
+ Misses       2661     2532     -129     
- Partials      727      780      +53     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hi everyone. I've tried to resolve the last checks failing. The doc won't build for some files because TomographyWithAstra only supports CUDA operations, this concerns:

• the code snippets in the docstring
• and the example in external-libraries/demo_astra_tomography.py

Is there a quick way to skip those checks if a CUDA device is not available ?

• in the doctest_global_setup I've added an import for astra and a check for cuda device, then the snippets in the docstring check for it and execute the code dependently
• I've renamed demo_astra_tomography.py -> _demo_astra_tomography.py, this way it is parsed but not executed.

Overall, I've implemented the tests and docs to be skipped if there is no GPU, I guess this is why the code coverage marks most of the implemented functions as not tested ?

Another point concerns the supported platform, at the moment, ASTRA only provides a pip package for linux. To get support for windows, you need to install it via their conda channel. I've restricted the dependency install to Linux platform for the moment.

One solution to the lack of CPU tests that we discussed with @tachella is to mock the Astra library calls, like @jscanvic does to create dataset tests when we can't actually download the datasets in #490. Do you think this could be possible?

The philosophy here is that as Astra is an external library, our tests should not have to cover its functionality, only our own wrapper. Then it is also ok that we don't have tests for adjointness etc, only that it runs ok and outputs correct shapes etc. Would you agree with this idea?

One solution to the lack of CPU tests that we discussed with @tachella is to mock the Astra library calls, like @jscanvic does to create dataset tests when we can't actually download the datasets in #490. Do you think this could be possible?

The philosophy here is that as Astra is an external library, our tests should not have to cover its functionality, only our own wrapper. Then it is also ok that we don't have tests for adjointness etc, only that it runs ok and outputs correct shapes etc. Would you agree with this idea?

It seems like a good alternative while there is no GPU for testing ! I'll dig into it and come up with a solution.

Although, I think keeping the already written tests in the codebase might be a good idea, at least it gives the ability to check on local machines that next versions of ASTRA or torch don't change the intented behaviour, in particular for the pseudo-inverse which is half torch, half astra code.

@Andrewwango I've patched the calls to the astra kernel with dummy functions in a new test, it should be good, tell me what you think. I put it in a test_external_libraries.py.

@romainvo Please can you check out the bugs in the workflows?