Skip to content
This repository has been archived by the owner on Jul 29, 2023. It is now read-only.

Fix model architecture for deployment to ONNX #234

Merged
merged 36 commits into from
May 29, 2023

Conversation

ziw-liu
Copy link
Contributor

@ziw-liu ziw-liu commented May 12, 2023

Addresses most of #214.

Also includes fixes to the inference module.

I'm opening this now for code visibility, but probably should merge after #232 (this is based on that branch).

@ziw-liu ziw-liu added bug Something isn't working enhancement New feature or request inference using and sharing the models labels May 12, 2023
@ziw-liu ziw-liu marked this pull request as ready for review May 15, 2023 21:52
mattersoflight and others added 5 commits May 17, 2023 17:28
* sync log metrics

* example of using more GPUs

* sync log metrics

* example of using more GPUs
this was accidentally tracked

The main command for inference is:

```buildoutcfg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior of inference CLI needs to be changed, I get following error when trying to run inference.

(lit-monai) [shalin.mehta@gpu-a-2 microDL]$ python micro_dl/cli/torch_inference_script.py --config /hpc/projects/CompMicro/projects/virtual_staining/models/phase2nuclei018/test/inference_config_processed_hek_no_perturb_512_512.yml 
Traceback (most recent call last):
  File "/hpc/mydata/shalin.mehta/code/microDL/micro_dl/cli/torch_inference_script.py", line 201, in <module>
    main(args.config, args.gpu, args.gpu_mem_frac)
  File "/hpc/mydata/shalin.mehta/code/microDL/micro_dl/cli/torch_inference_script.py", line 190, in main
    torch_predictor = torch_inference_utils.TorchPredictor(
  File "/hpc/mydata/shalin.mehta/code/microDL/micro_dl/inference/inference.py", line 59, in __init__
    self.network_z_depth = self.network_config["in_stack_depth"]
KeyError: 'in_stack_depth'

I think we are now linking network_z_depth and in_stack_depth in train.py

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's switch to lightning CLI's predict subcommand after we merge this PR.

## Exporting models to onnx
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deployment script uses inference module and I am seeing the same error as above:

(lit-monai) [shalin.mehta@gpu-a-2 microDL]$ python micro_dl/cli/onnx_export_script.py --model_path /hpc/projects/CompMicro/projects/virtual_staining/models/phase2nuclei018/lightning_logs/20230514-003340/checkpoints/epoch=29-step=4350.ckpt --stack_depth 5 --export_path /hpc/projects/CompMicro/projects/virtual_staining/models/phase2nuclei018/deployment/20230514-003340_epoch29.onnx 
Initializing model in pytorch...
Traceback (most recent call last):
  File "/hpc/mydata/shalin.mehta/code/microDL/micro_dl/cli/onnx_export_script.py", line 177, in <module>
    main(args)        
  File "/hpc/mydata/shalin.mehta/code/microDL/micro_dl/cli/onnx_export_script.py", line 164, in main
    export_model(model_dir, model_name, args.stack_depth, args.export_path)
  File "/hpc/mydata/shalin.mehta/code/microDL/micro_dl/cli/onnx_export_script.py", line 99, in export_model
    torch_predictor = inference.TorchPredictor(
  File "/hpc/mydata/shalin.mehta/code/microDL/micro_dl/inference/inference.py", line 59, in __init__
    self.network_z_depth = self.network_config["in_stack_depth"]
KeyError: 'in_stack_depth'

Copy link
Member

@mattersoflight mattersoflight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ziw-liu with the change in training config, the inference and export CLI are throwing an error that seems easy to fix. Please handle them both. Feel free to update the inference stage to use predict subcommand of lightning CLI. That may impose some directory structure, but that is a good thing! We can handle the refactor of inference CLI as the next PR if that is easier for you.

@ziw-liu
Copy link
Contributor Author

ziw-liu commented May 27, 2023

@mattersoflight I was already moving inference into lightning (#236) as you reviewed this PR. Can we merge this as-is?

@mattersoflight mattersoflight merged commit 00375b0 into pytorch_implementation May 29, 2023
1 check failed
ziw-liu added a commit that referenced this pull request May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working enhancement New feature or request inference using and sharing the models
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants