Maped torch implementation#256
Conversation
|
@henrygbell beautiful github workflow. clean simple, thanks for the tests. I will review later this week! |
|
Quick comment on the class naming: Presumably the ambition is to make most (all) quantem functionality torch-first, so I would avoid class names like |
bobleesj
left a comment
There was a problem hiding this comment.
@henrygbell left comments for strengthening API doc to guide humans and some minor formatting
|
|
||
| return self | ||
|
|
||
| def real_space_align( |
There was a problem hiding this comment.
we need a good documentation on which parameters (and magnitude) to use. are there any chances where these parameters matter? these human lessons should be documented in general.
p.s. the goal is to not using/writing any parameters when possible - all automated if possible. but that's not really possible in practice.
There was a problem hiding this comment.
generally for all other function as well - scientists wll run the pubic API in jupyter notebook, they find their results are bad, then they will revert to this API doc,
we want this API doc to provide clues/lessons for them. - smae goes for LLM/AI.
| return out | ||
|
|
||
|
|
||
| class MAPEDTorch(AutoSerialize): |
There was a problem hiding this comment.
as @gvarnavi mentioned - MAPED, don't think we have to call it Maped, etc. MAPED
| padding_mode="zeros", | ||
| align_corners=True, | ||
| ) | ||
|
|
There was a problem hiding this comment.
try not to have any extra empty lice spaces - we want code to read like paragraphs, each paragraph, each thesis/reason to exist.
| device = imageCorr.device | ||
| _, M, N = imageCorr.shape | ||
| pixelRadius = 1.5 | ||
| numRow = int(math.ceil(pixelRadius * upsampleFactor)) |
There was a problem hiding this comment.
numRow, row_coords, etc. keep naming consistent throughout
| ) | ||
|
|
||
| # log-parabolic sub-pixel refinement — col direction | ||
| col_safe = col_peak.clamp(1, W_dp - 2) |
There was a problem hiding this comment.
yup, as mentioned, this is a good code block where no extra line spaces, but only add one when there is a reason to add.
…mendations in docstrings, added docstring for dscan_align.
|
@bobleesj Thanks for your comments. They have been addressed in the recent commits, I also updated the validation notebook for the MAPEDTorch -> MAPED change: |
What problem does this PR address?
The existing MAPED class (#169) runs on NumPy/CPU, which makes the alignment + merge pipeline slow on the large 4D-STEM tilt/precession stacks it's meant for. This PR adds MAPEDTorch, a PyTorch port of the full MAPED workflow that runs on GPU (or CPU) and operates directly on torch.Tensor 4D-STEM volumes.
[Describe the problem this PR solves. Focus on inputs/outputs.
Attach screenshots, plots, and before/after comparisons.]
[Show the function signatures, class interfaces, and how a scientist
would use this in a notebook. The API is what a scientist actually
types. Get this right first.]
This PR implements the following methods:
API — how a scientist uses it in a notebook:
What should the reviewer(s) do?
Attached is a validation notebook which executes the whole pipeline against ground truth and is the easiest way to review correctness: maped_torch_validation.ipynb
Validation notebook workflow:
Current results of this PR's MAPED code
Please use this notebook for future updates to the MAPED pipeline to assess correctness.
Please look at:
If possible: