direct predict option for inference and minor fixes
#151
Conversation
No longer needed. Funcionality now covered by `direct predict`.
| common_parser = Args(add_help=False) | ||
| predict_parser = parser.add_parser( | ||
| "predict", | ||
| help="Run inference using direct.", | ||
| parents=[common_parser], | ||
| epilog=epilog, | ||
| formatter_class=argparse.RawDescriptionHelpFormatter, | ||
| ) | ||
| predict_parser.add_argument("data_root", type=pathlib.Path, help="Path to the inference data directory.") | ||
| predict_parser.add_argument("output_directory", type=pathlib.Path, help="Path to the output directory.") | ||
| predict_parser.add_argument( | ||
| "experiment_directory", | ||
| type=pathlib.Path, | ||
| help="Path to the directory with checkpoints and config.", | ||
| ) | ||
| predict_parser.add_argument( | ||
| "--checkpoint", | ||
| type=int, | ||
| required=True, | ||
| help="Number of an existing checkpoint in experiment directory.", | ||
| ) | ||
| predict_parser.add_argument( | ||
| "--filenames-filter", | ||
| type=pathlib.Path, | ||
| help="Path to list of filenames to parse.", | ||
| ) | ||
| predict_parser.add_argument( | ||
| "--name", | ||
| dest="name", | ||
| help="Run name if this is different than the experiment directory.", | ||
| required=False, | ||
| type=str, | ||
| default="", | ||
| ) | ||
| predict_parser.add_argument( | ||
| "--cfg", | ||
| dest="cfg_file", | ||
| help="Config file for inference. Can be either a local file or a remote URL." | ||
| "Only use it to overwrite the standard loading of the config in the project directory.", | ||
| required=False, | ||
| type=file_or_url, | ||
| ) | ||
|
|
||
| predict_parser.set_defaults(subcommand=predict_from_argparse) |
There was a problem hiding this comment.
Is this all required for a predict script? Ideally shouldn't you ingest a file (or a number of files)? That'd be more useful, so don't use filenames_filter, but take a filename or a regex. Would that work? The --checkpoint can be replaced by --checkpoint-no and --checkpoint-path where the latter accepts a full path. We can additionally use environmental variables to do it.
setup.py
Outdated
| @@ -46,6 +46,7 @@ | |||
| "ismrmrd==1.9.1", | |||
| "tensorboard>=2.5.0", | |||
| "tqdm", | |||
| "sewar", | |||
There was a problem hiding this comment.
Perhaps don't add sewar, it's only for the VIF. You can use a mechanism such as https://github.com/NKI-AI/dlup/blob/main/dlup/utils/imports.py to check if it is there and otherwise give a RunTimeError when you try to run the Calgary Campinas or something like that?
There was a problem hiding this comment.
No I don't really agree as you may want to run C-C but withouf VIF. Maybe if "calgary_campinas_vif_metric" is requested as an evaluation metric.
There was a problem hiding this comment.
Yes. But then it must crash before it is at the validation step. E.g. when building metrics.
Codecov Report
@@ Coverage Diff @@
## main #151 +/- ##
=======================================
Coverage ? 28.28%
=======================================
Files ? 80
Lines ? 6647
Branches ? 0
=======================================
Hits ? 1880
Misses ? 4767
Partials ? 0 Continue to review full report at Codecov.
|
direct predictoption for inference from cli