AdvancedPhotonSource / tike

Repository for ptychography software
http://tike.readthedocs.io
Other
29 stars 15 forks source link

NEW: Implement multislice propagation operator with correct adjoint #316

Closed carterbox closed 4 months ago

carterbox commented 5 months ago

Purpose

Implement a correct adjoint operation for multi-slice propagation.

Approach

Pre-Merge Checklists

Submitter

Reviewer

pep8speaks commented 5 months ago

Hello @carterbox! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 107:81: E501 line too long (85 > 80 characters)

Line 32:81: E501 line too long (83 > 80 characters) Line 70:81: E501 line too long (81 > 80 characters) Line 71:81: E501 line too long (81 > 80 characters) Line 72:81: E501 line too long (81 > 80 characters) Line 97:81: E501 line too long (83 > 80 characters)

Line 32:81: E501 line too long (83 > 80 characters) Line 69:81: E501 line too long (81 > 80 characters) Line 70:81: E501 line too long (81 > 80 characters) Line 71:81: E501 line too long (81 > 80 characters)

Comment last updated at 2024-06-05 19:32:16 UTC
a4894z commented 4 months ago

I was trying to actually run these changes on some benchmarking data, but it was ending up looking like the multislice_fwd_infrastructure branch which kinda was defeating the point of splitting the PR up like this.

By eye, it looks OK but I haven't tested it in ePIE yet.

carterbox commented 4 months ago

Thanks, @a4894z. I think the way to test this PR was to just run the included adjoint tests. Reading through the changes to check for understandable documentation and typos and copypasta are also helpful.