Add position prediction #24
Conversation
ptychonn/pospred/configs.py
Outdated
| class InferenceConfigDict(ConfigDict): | ||
|
|
||
| # ===== PtychoNN configs ===== | ||
| batch_size: Any = 1 |
There was a problem hiding this comment.
What's the purpose of providing type hints, if everything is type "Any"?
There was a problem hiding this comment.
They were placeholders when I refactored it from defaultdict to dataclass and will be filled in in the next commit.
|
Comments have been addressed as of commit 41faf63. |
| self.random_seed = 123 | ||
| self.tol = configs.nonhybrid_registration_tol | ||
| self.min_roi_stddev = configs.min_roi_stddev | ||
| self.debug = False |
There was a problem hiding this comment.
Why not use the python built-in __debug__ instead of adding a parameter this object?
There was a problem hiding this comment.
My knowledge might be incomplete and inaccurate, but apparently __debug__ is True by default and the only way to turn it off is to run the script with -O or set the PYTHONOPTIMIZE environment variable. This is opposite to what I wanted since I want the debugging code to not run by default.
ptychonn/position/configs.py
Outdated
|
|
||
| schedule_learning_rate: bool = True | ||
|
|
||
| pretrained_model_path: str = None |
There was a problem hiding this comment.
| pretrained_model_path: str = None | |
| pretrained_model_path: typing.Optional[str] = None |
Any parameter that has None as the default value is Optional.
There was a problem hiding this comment.
Since this is now in the PtychoNN package, modeling training and inference should use the protocols that already exist in the package, so I removed the parameters related to PtychoNN training and inference in f5a3af2.
ptychonn/position/configs.py
Outdated
| num_lines_for_validation: int = 805 | ||
| """Number of lines used for testing""" | ||
|
|
||
| dataset: Any = None |
There was a problem hiding this comment.
How can the dataset be optional? Maybe you want to move this parameter to the top and make it required by not providing a default value?
There was a problem hiding this comment.
Similar to the comment above, since this is now a part of the PtychoNN package, it should use the existing protocols for training and inference so I removed the config parameters in the position module in f5a3af2.
There was a problem hiding this comment.
No parameters should have a type hint of Any unless they can truly be anything. Parameters that can be int or None, for example, should either be typing.Optional[int] or int | None. The | syntax requires a minimum python version of 3.10.
There was a problem hiding this comment.
Replaced Anys with more specific type hints in fdc7ae7.
ptychonn/position/reconstructor.py
Outdated
| class Reconstructor: | ||
| def __init__(self, config_dict: InferenceConfig): | ||
| """ | ||
| Inference engine for PtychoNN. | ||
|
|
||
| :param config_dict: dict. Configuration dictionary. | ||
| """ | ||
| self.config_dict = config_dict | ||
| self.device = None | ||
|
|
||
| def build(self): | ||
| if torch.cuda.is_available(): | ||
| self.device = torch.device("cuda") | ||
| else: | ||
| self.device = torch.device("cpu") | ||
|
|
||
| def batch_infer(self, x): | ||
| pass |
There was a problem hiding this comment.
Can unused definitions like this one be removed?
There was a problem hiding this comment.
Removed unused definitions and modules in bc80a68.
|
LGTM. Thanks, @mdw771! |
Added feature
Position prediction module is added in
ptychonn/pospred.Usage
See
pospred_readme.md.Test
A unit tester is added as
tests/test_multiiter_pos_calculation.py. The tester takes predicted images and configurations fromtests/data/pospred/pred_test235andtests/data/pospred/config_235.tomland performs a 3-iteration run of position prediction. Results are compared with reference data intests/data_gold/pospred/calc_pos_235.csv.