Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Brain LDM to model-zoo #188

Merged
merged 22 commits into from
Mar 1, 2023

Conversation

Warvito
Copy link
Collaborator

@Warvito Warvito commented Jan 14, 2023

Implements #30

Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@Warvito Warvito linked an issue Jan 14, 2023 that may be closed by this pull request
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@Warvito
Copy link
Collaborator Author

Warvito commented Jan 14, 2023

@ericspod probably for the metadata of generative models (example here), we will have schemas different from the ones availabe in MONAI-extra-test-data . Should we create an issue in https://github.com/Project-MONAI/MONAI-extra-test-data to include new schemas?

Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@ericspod
Copy link
Member

@Warvito We might have to add a new schema for now yes. What I had wanted for a more ideal solution was a schema that was general enough to cover any input/output definition for a model, so something that would allow us to define an arbitrary set of inputs that are tensors with the given data or some other Python object, and similar for outputs. Right now we're limited to explicitly-named arguments and outputs since that's the intial way we figured how to do the schema.

If someone knows more about JSON schemas wanted to figure out how to generalise things we could get a schema to suit everyone.

Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@Warvito
Copy link
Collaborator Author

Warvito commented Feb 14, 2023

Hi @ericspod , I was trying to write the bundle for this model, but due its particular inference method, it would not be possible to use the inferer's method. Is it possible to define for loops inside the bundle? would you have an example of it?

@ericspod
Copy link
Member

ericspod commented Feb 15, 2023

I wouldn't suggest trying to do things directly in the bundle due to the limitations of the format, it's perhaps possible but would be really ugly Python code to make it happen. The MedNIST GAN bundle illustrates defining a script directory where you can put your own code, I would put a routine into a source file there which does the inference you want with the network, scheduler, etc. as arguments. @Warvito

@Warvito
Copy link
Collaborator Author

Warvito commented Feb 15, 2023

I wouldn't suggest trying to do things directly in the bundle due to the limitations of the format, it's perhaps possible but would be really ugly Python code to make it happen. The MedNIST GAN bundle illustrates defining a script directory where you can put your own code, I would put a routine into a source file there which does the inference you want with the network, scheduler, etc. as arguments. @Warvito

Thanks for the help. I will it ^^

Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@Warvito Warvito marked this pull request as ready for review February 27, 2023 18:11
Copy link
Collaborator

@marksgraham marksgraham left a comment

Choose a reason for hiding this comment

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

Hi Walter, this is really cool, congrats!
The only issue for me is output_dir not getting created. Do you find this on your end? If not let me know and I'll try to fix on my end - maybe its something weird about my system.

Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@Warvito Warvito requested a review from marksgraham March 1, 2023 09:49
Copy link
Collaborator

@marksgraham marksgraham left a comment

Choose a reason for hiding this comment

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

All works now!

@marksgraham marksgraham merged commit 222a8f1 into main Mar 1, 2023
@marksgraham marksgraham deleted the 30-add-pretrained-model-on-brain-data-to-model-zoo branch March 1, 2023 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pretrained model on brain data to model zoo
3 participants