Create a command line app to generate config files from argparse. about deepreg HOT 26 CLOSED

This is related to #36 .

from deepreg.

ok - agree on CV.
We have chatted about different versions of this CLI - i'm a bit losing the track now - but definitely no conclusion reached - another item for thursday maybe?

from deepreg.

@YipengHu Later i will provide a doc on what i imagined for cli tool for config generation.

from deepreg.

It is helpful for the user from my current understanding. I am not sure actually.
I can proceed anyway if we decide to go.

from deepreg.

@NMontanaBrown could you remind us what are we expecting exactly? Would you describe a simple use case?

@YipengHu I suggest to wait for further details.

Concerning onboarding @ZheMin-1992, if you are interested, we have some other tf-related issues like #468 #469 ;) and I can onboard you on these issues.

from deepreg.

ok, so it's more like a flattened version of config? yeah, good to have for users maybe.

from deepreg.

Shall I assign myself this ticket to clarify? May be more helpful to address since I have the vision of what this generically would look like! @YipengHu @mathpluscode

from deepreg.

Shall I assign myself this ticket to clarify? May be more helpful to address since I have the vision of what this generically would look like! @YipengHu @mathpluscode

Agree ;) this sounds better.

from deepreg.

A question for @mathpluscode, as I design this tool. Do we /need/ all the models in the config? Is this some weird dependency that needs fixing where all three must be present or am I missing something here?

DeepReg/config/unpaired_labeled_affine.yaml

Line 12 in 7e5faf1

method: "affine" # ddf or conditional or seg

In this yaml example there are three defined (local, global and unet). Surely we only need params for one model?

No, we should just need what is necessary. I left them there so that it's easier to switch lol... @NMontanaBrown

from deepreg.

Reviving this from the dead @mathpluscode are we planning on this for rc1?

I really do not think this is important/urgent... lol I would even prefer drop this.

from deepreg.

I am closing this unless there is going to be any immediate update.

from deepreg.

Porting doc request to that ticket, this ticket will be about creating CLI for the generation of config files. @YipengHu

from deepreg.

@NMontanaBrown @agrimwood not sure i follow this line of work closely. If this is still ongoing, it would be handy to add one more functionality to generate the cross-validation "batch" config files. See this: https://ucl-candi.github.io/DeepReg/#/tutorial_experiment
This should be relatively independent to other code/tests but, again, is optional for R1.

from deepreg.

Hi @YipengHu, as evidenced by lack of commits this ticket has not yet been addressed. There have been many changes to the config situation so mostly just waiting for the dust to settle before building a solution - if the config docs are done and there are no more expected changes to config situation then we can work on this. @agrimwood is currently working on testing so we can re-distribute this ticket.

from deepreg.

Hi @NMontanaBrown , what do you mean about "own configuration files to customise trainings"?

I thought first you wanted to have a CLI to merge configs, but seems not exactly this?

The current system would work, if user copy a config and change the values themselves in that copy, then pass the config path.

But maybe you want something a bit more modular, and user can create a config by answering questions like

1. is your data labeled? Y/N
2. which model you want to choose? 1) local net, 2) global net, etc.
   etc.

What do you think? @YipengHu @NMontanaBrown

About the cross validation, maybe another issue lol

from deepreg.

ok - agree on CV.
We have chatted about different versions of this CLI - i'm a bit losing the track now - but definitely no conclusion reached - another item for thursday maybe?

Since we have decided to do this, should we re-visit generating multiple config files for CV? I thought we agreed on this during the meeting.

from deepreg.

I'm marking this post-r1 now.

from deepreg.

@ZheMin-1992 can you review this and give an estimates of the time required? thanks

from deepreg.

I'd suggest to hold on this or even close it... The task is not clearly defined and I would not say it's of high priority, same as #301 @YipengHu

from deepreg.

I'd suggest to hold on this or even close it... The task is not clearly defined and I would not say it's of high priority, same as #301 @YipengHu

@ZheMin-1992 what do you think?

from deepreg.

It is helpful for the user from my current understanding. I am not sure actually.
I can proceed anyway if we decide to go.

Let's proceed.

from deepreg.

Hi, yes, I am now back from the dead

@mathpluscode - "The current system would work, if user copy a config and change the values themselves in that copy, then pass the config path."

My idea was to auto generate this from command line such that:

python deepreg_cli_tool_config --path /path/to/save --net "Unet" --loss "X" --batch 32 --opt "Adam" etc etc (I forgot the args in the config, but hopefully you get the gist)

generates a config file in /path/to/save that is exactly what deepreg needs to train the nets.

Basically a simple module that "fills in" a config file template and "guides" the user by providing all the fields that need to filled in with their parameters.

This means that you can batch generate config files using bash scripts, or just avoid copy pasting as many people that work with command line do.

thoughts @YipengHu @ZheMin-1992

from deepreg.

a tool like that would be handy - more about a good design.

from deepreg.

A question for @mathpluscode, as I design this tool. Do we /need/ all the models in the config? Is this some weird dependency that needs fixing where all three must be present or am I missing something here?

In this yaml example there are three defined (local, global and unet). Surely we only need params for one model?

from deepreg.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

from deepreg.

Reviving this from the dead @mathpluscode are we planning on this for rc1?

from deepreg.