uellue commented 3 years ago

Fast image transforms using a sparse matrix product
Coordinate transforms to match detector data with forward model
Calculate stacks of shifted and rolled products of a smaller illumination within a larger object, including sub-pixel shifting
Aggregate shifted and rolled updates within a larger object

The functions can perform FFT shifts on the fly to reduce memory transfers. Both CPU and CUDA are supported. They are tested to conform precisely to the conventions of libertem.api.create_com_analysis() and also SSB_UDF.

CC @bangunarya @Sniper2k that's the stuff I have mentioned recently. If you'd like to try it, let me know! :-)

Contributor Checklist:

[x] I have added myself to the creators.json file
[x] I have added a changelog entry for my contribution
[x] I have added/updated documentation for all user-facing changes
[x] I have added/updated test cases

SimeonEhrig commented 3 years ago

@uellue Is there are a rendered version of the sphinx doc available? In the alpaka-group, we use readthedoc. There is a CI job, which build the sphinx doc and provides an link to the result.

uellue commented 3 years ago

@uellue Is there are a rendered version of the sphinx doc available? In the alpaka-group, we use readthedoc. There is a CI job, which build the sphinx doc and provides an link to the result.

At the moment the rendered documentation is only available after merging or by running tox -e docs locally. @sk1p, having the rendered docs of a PR available could help with reviewing, since sometimes it builds cleanly but looks terrible. I've reviewed the rendered docs locally up to now, which is a bit tedious.

SimeonEhrig commented 3 years ago

At the moment the rendered documentation is only available after merging or by running tox -e docs locally. @sk1p, having the rendered docs of a PR available could help with reviewing, since sometimes it builds cleanly but looks terrible. I've reviewed the rendered docs locally up to now, which is a bit tedious.

Thanks. I was not sure, if the CI job provides an artifact of the build. But building locally is also fine.

codecov[bot] commented 3 years ago

Codecov Report

Merging #60 (ef0c3e6) into master (fec553e) will decrease coverage by 4.15%. The diff coverage is 53.98%.

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
- Coverage   64.08%   59.92%   -4.16%     
==========================================
  Files           8        8              
  Lines         323      549     +226     
  Branches       38       66      +28     
==========================================
+ Hits          207      329     +122     
- Misses        104      206     +102     
- Partials       12       14       +2

Impacted Files	Coverage Δ
src/ptychography40/reconstruction/common.py	`54.54% <53.98%> (-2.13%)`	:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update fec553e...ef0c3e6. Read the comment docs.

sk1p commented 3 years ago

It looks like the current mac os x image doesn't include Python 3.6 anymore - we may need to switch to an older one (at least for 3.6). See here: https://github.com/actions/virtual-environments/blob/main/images/macos/toolsets/toolset-10.14.json https://github.com/actions/virtual-environments/blob/main/images/macos/toolsets/toolset-11.json

uellue commented 3 years ago

It looks like the current mac os x image doesn't include Python 3.6 anymore - we may need to switch to an older one (at least for 3.6). See here: https://github.com/actions/virtual-environments/blob/main/images/macos/toolsets/toolset-10.14.json https://github.com/actions/virtual-environments/blob/main/images/macos/toolsets/toolset-11.json

Should I do sth about that in this PR? Can you give me a pointer?

sk1p commented 3 years ago

Should I do sth about that in this PR? Can you give me a pointer?

You could try to set the vmImage in the mac os x matrix to macos-10.14 for the Python36 entry here:

https://github.com/Ptychography-4-0/ptychography/blob/18114c5103ceff0ac360a1be2a405ddb1c9bbfee/azure-pipelines.yml#L192-L202

Should be possible to just specify vmImage as sibling of TOXENV and python.version, like this example.

sk1p commented 3 years ago

Hm, that didn't work - still showing "Image: macOS-latest"...

uellue commented 3 years ago

@sk1p is the notebook that shows the transforms understandable? Do we need more documentation, are there other things to address? It would be nice to merge this some time next week to make it easier to use in downstream code. :-)

sk1p commented 3 years ago

is the notebook that shows the transforms understandable?

Yes, thanks!

Do we need more documentation, are there other things to address? It would be nice to merge this some time next week to make it easier to use in downstream code. :-)

I think documentation can always be improved, but let's merge this now (perfect is the enemy of good, and all that...)

Ptychography-4-0 / ptychography

Utility functions for iterative algorithms #60

Contributor Checklist:

Codecov Report