Added Mikula finetuning script #132
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@oumayb Could you maybe add the config files as well, so that we can see the differences between |
|
@mathieuboudreau sure! |
|
@oumayb I don't know if the trained models should all be included here – are the files large? As for config files, it may be worth having a separate folder (e.g. axondeepseg/configs/) that we can sort/store the config files separately from the core source code (currently SEM/TEM config files are inside the source code folder axondeepseg/AxonDeepSeg/models/). @jcohenadad thoughts? |
|
I would not copy Mikula's model under the GH's repository, because (i) it is still preliminary and (ii) we should instead store them somewhere else (along with the config files, which I believe should be side-by-side with the model). New issue here: #133. @oumayb could you please indicate where the model currently is? |
|
@jcohenadad & @mathieuboudreau for the moment the trained models & config files are located in |
|
@oumayb I ran your Jupyter Notebook with the data located in The errors I got were not always the same each time I tried running the training, differening slightly every time. See the following for three cases:
And see below for the full error logs: Run 1
Run 2
Run 3
@oumayb do you know what might be causing this error? Maybe @perone too? |
|
@mathieuboudreau this usually happens when we're finetuning a model using another one that doesn't have the exact same architecture, so I think the problem comes from the config files that were used. I'll look more into it and get back to you! |
|
@oumayb Ok that makes sense – I was wondering how we could have reused the TEM model to finetune the SEM one, which has a different architechture. Thanks! |
|
@mathieuboudreau yes exactly, they differ in the depth and in the number of convolutions per layer, which should be taken into account in the config files. |
|
@oumayb Ok! That wasn't clear to me when reading the Jupyter Notebook + using the files that @alexfoias suggested. If you kept the most up-to-date files somewhere, please let me know! |
|
@mathieuboudreau sure! I'll get back to you soon with the files |
|
@mathieuboudreau |
|
Thanks @oumayb ! That helped me get a little bit further, but now I'm encountering another error:
It looks like it has trouble opening the evolution file of the old model that I'm trying to finetune from. You didn't indicate where the model you were finetuning from was located (just its name @oumayb or @perone have you encountered this issue? Not sure if this is a Python 2 to 3 conversion functionality difference (seems like file is not being loaded in byte mode by default?). I tried opening the file with these lines below instead on the command line, which works, but want to make sure that this is a bug before trying to fix it: @oumayb which version of ADS were you using when doing your finetuning work, do you know? Were you using the Python 2 or Python 3 version? |
|
Hmm, loaded using the lines above doesn't seem to work correctly, as the field "steps" isn't loaded, which is called in the next line. There must be something that else I'm missing that you were doing @oumayb ? |
|
Ok, finally got it working. You must have been using the Python 2 version of ADS when originally training your finetuned models. The old model which we're finetuning from was saved using Python 2, and there is a mismatch in how pickle deals with the pickling/unpickling files (and in particular, numpy arrays) between Python 2/3. I think I dealt with this once before while writing the unit tests, but the lines mentioned above were some of the few that don't have coverage yet, so this bug existed since the Python 3 upgrade. I had to change these lines: to this: and then the training started successfully. I'll have to make a similar change, write a test(s), and commit it. The lines above might be enough, I just need to check to make sure they would work correctly with an evolution file pickled in Python 3 as well. |
|
Thank you so much for taking care of that @mathieuboudreau!!! 👍 |
|
I think this PR has gotten too stale to merge. In particular, I think @vs74's new Keras implementation means we'll need to use a different way to finetune. While we should still keep this PR in consideration moving forward, I'm going to close it for now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Notebook for mikula dataset