Integrate custom datastore #30
Conversation
…elated to that (and batch submission to remote cluster) to a subfolder ./datastoreworkflow
With this update, the custom datastore will accept an option struct. Currently, the (optional) fields in this option struct are: - doAutoResize - numberOfFlankingFrames - outputFrameSize
|
@theavuik I think the branch I pushed is in no good state to be merged. Is that what you are referring to? I think what @jbentvelsen is doing is ok (at least for me) |
There was a problem hiding this comment.
After denoising one frame, I obtain a figure with two subfigs (raw and denoised). After denoising all frames I do not see any figures, is that OK?
Is it possible to add the 'raw' and 'denoised' also to the bottom figures related to the for-loop?
There was a problem hiding this comment.
@theavuik that was intended in the way the deepIntInference example function was written, it will denoise and then save the stack to disc when not given a frame number. However, this functions is not exactly connected to the custom datastore and should be removed, it does not even use the custom datastore. The "deepIntTraining" function I submitted showed training using the custom datastore. Should we/I better create a revised workflow demo?
There was a problem hiding this comment.
@thkupy You are right, the current inference we see in the new customdatastore_example.mlx is not related to the datastore.
Maybe, the solution is that we simply remove the inference steps from the customdatastore_example? Such that the example ends after showing how to train a network with the custom datastore (these steps were based on your deepIntTraining function). I believe the other live scripts show how to do inference anyway, and the datastore is not relevant for inference.
|
Hi -
Just to confirm, you all are still discussing this PR and I shouldn’t test yet, true?
Thanks
Steve
… On Oct 17, 2023, at 6:14 AM, Thomas Künzel ***@***.***> wrote:
@tkuenzmw commented on this pull request.
On examples/customdatastore_example.mlx <#30 (comment)>:
@theavuik <https://github.com/theavuik> that was intended in the way the deepIntInference example function was written, it will denoise and then save the stack to disc when not given a frame number. However, this functions is not exactly connected to the custom datastore and should be removed, it does not even use the custom datastore. The "deepIntTraining" function I submitted showed training using the custom datastore. Should we/I better create a revised workflow demo?
—
Reply to this email directly, view it on GitHub <#30 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOOPDPR32FOMKNLJITWHHLX7ZLAPAVCNFSM6AAAAAA6CB6OXWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMOBRHEZTCMBXG4>.
You are receiving this because your review was requested.
|
I expect Joris will take a look at our comments first, but he is on holiday now. Thanks! |
jbentvelsen
force-pushed
the
integrate-custom-datastore
branch
from
63a048b to
0418ca8
Compare
As the folder structure is different, the git history will be lost anyway. Furthermore, we probably remove this function in a next version. So for now, I will leave this as it is. |
|
I have processed the feedback (renamed a variable and removed the inference part). @stevevanhooser (maybe also @tkuenzmw) can you review this PR? |
I will not formally review the PR but I'll comment here: I think this is now really good! In the future a workflow example for just the datastore workflow could maybe be even more reduced/simpler. For example the construction of the exampleOpts (ll. 2-13) is maybe a bit confusing here -- why are the parameters put into a struct at all, this is not clear to the "viewer". However, this is a minor point. I think you greatly improved the custom datastore and the demo related to it. Good stuff! |
jbentvelsen
force-pushed
the
integrate-custom-datastore
branch
from
d8396f2 to
4ec9031
Compare
|
I have added a section showing inference using the custom datastore (based on the code of @tkuenzmw). |
This PR will integrate the custom datastore into the DeepInterpolation repo. @tkuenzmw developed a first version of this datastore including an example. I have made some modifications such that the hard coded parameters are replaced by parameters. Also, I have extended the live script with some additional information about the this custom datastore.