Call NumpyToTensor last #818
Conversation
Pull Request Test Coverage Report for Build 947096899
💛 - Coveralls |
And from the documentation and tutorials, thanks |
ivadomed/transforms.py
Outdated
| if isinstance(tr, NormalizeInstance): | ||
| numpy_to_tensor = NumpyToTensor() | ||
| sample, metadata = numpy_to_tensor(sample, metadata) |
There was a problem hiding this comment.
Thanks @cakester.
I've changed my mind: instead of before the NomarlizeInstance, could you please force the NumpyToTensor as last transform?
Reason: The current solution would not work if, for some reason, the user do not use NormalizeInstance --> we still need our samples to be moved from Numpy to Tensor.
There was a problem hiding this comment.
I tried moving NumpyToTensor last, it was failing a bunch of tests then I tried
numpy_to_tensor_called = False
for tr in self.transform[data_type].transforms:
if isinstance(tr, NormalizeInstance):
sample, metadata = numpy_to_tensor(sample, metadata)
numpy_to_tensor_called = True
sample, metadata = tr(sample, metadata)
if not numpy_to_tensor_called:
sample, metadata = numpy_to_tensor(sample, metadata)
return sample, metadata
and looked at the test results, the test_automated_training fails nevertheless (same failure message), mind taking a look as I do not know how to resolve that error? thanks
There was a problem hiding this comment.
Can you please copy the error in this thread? Cheers
But my guess is: Have you removed the NumpyToTensor from the config file used by the testing? If not --> that's the reason: this transform is called twice: you get an error on the second call because the sample is already a Tensor.
The solution you implemened looks good. But you now need to remove NumpyToTensor from all the config files, documentation and testing. Cheers
There was a problem hiding this comment.
I tried removing NumpyToTensor from automate_training_config.json and I got this
--- Logging error ---
Traceback (most recent call last):
File "/Users/runner/work/ivadomed/ivadomed/ivadomed/scripts/automate_training.py", line 95, in train_worker
ivado.run_command(config, thr_increment=thr_incr)
File "/Users/runner/work/ivadomed/ivadomed/ivadomed/main.py", line 407, in run_command
debugging=context["debugging"])
File "/Users/runner/work/ivadomed/ivadomed/ivadomed/training.py", line 158, in train
for i, batch in enumerate(train_loader):
File "/Users/runner/hostedtoolcache/Python/3.7.10/x64/lib/python3.7/site-packages/torch/utils/data/dataloader.py", line 345, in __next__
data = self._next_data()
File "/Users/runner/hostedtoolcache/Python/3.7.10/x64/lib/python3.7/site-packages/torch/utils/data/dataloader.py", line 385, in _next_data
data = self._dataset_fetcher.fetch(index) # may raise StopIteration
File "/Users/runner/hostedtoolcache/Python/3.7.10/x64/lib/python3.7/site-packages/torch/utils/data/_utils/fetch.py", line 44, in fetch
data = [self.dataset[idx] for idx in possibly_batched_index]
File "/Users/runner/hostedtoolcache/Python/3.7.10/x64/lib/python3.7/site-packages/torch/utils/data/_utils/fetch.py", line 44, in <listcomp>
data = [self.dataset[idx] for idx in possibly_batched_index]
File "/Users/runner/work/ivadomed/ivadomed/ivadomed/loader/loader.py", line 754, in __getitem__
data_type="im")
File "/Users/runner/work/ivadomed/ivadomed/ivadomed/transforms.py", line 151, in __call__
sample, metadata = tr(sample, metadata)
File "/Users/runner/work/ivadomed/ivadomed/ivadomed/transforms.py", line 50, in wrapper
return wrapped(self, sample, metadata)
File "/Users/runner/work/ivadomed/ivadomed/ivadomed/transforms.py", line 76, in wrapper
return wrapped(self, sample, metadata)
File "/Users/runner/work/ivadomed/ivadomed/ivadomed/transforms.py", line 889, in __call__
metadata["elastic"] = [None, None]
TypeError: list indices must be integers or slices, not str
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "c:\users\hanna.ma\appdata\local\programs\python\python38\lib\runpy.py", line 194, in _run_module_as_main
return _run_code(code, main_globals, None,
File "c:\users\hanna.ma\appdata\local\programs\python\python38\lib\runpy.py", line 87, in _run_code
exec(code, run_globals)
File "C:\Users\hanna.ma\AppData\Local\Programs\Python\Python38\Scripts\ivadomed_automate_training.exe\__main__.py", line 7, in <modu
le>
File "c:\users\hanna.ma\appdata\local\programs\python\python38\lib\site-packages\ivadomed\scripts\automate_training.py", line 790, i
n main
automate_training(file_config=args.config,
File "c:\users\hanna.ma\appdata\local\programs\python\python38\lib\site-packages\ivadomed\scripts\automate_training.py", line 704, i
n automate_training
validation_scores = pool.map(partial(train_worker, thr_incr=thr_increment), config_list)
File "c:\users\hanna.ma\appdata\local\programs\python\python38\lib\multiprocessing\pool.py", line 364, in map
return self._map_async(func, iterable, mapstar, chunksize).get()
File "c:\users\hanna.ma\appdata\local\programs\python\python38\lib\multiprocessing\pool.py", line 771, in get
raise self._value
TypeError: list indices must be integers or slices, not str
There was a problem hiding this comment.
I tried removing NumpyToTensor from automate_training_config.json and I got this
Which commit? Can't see. Thanks
There was a problem hiding this comment.
I ran locally with the commit that's checked into github and this was what I got after removing NumpyToTensor from automate_training_config.json, data_functional_testing is downloaded it is not checked into git therefore I currently do not know how to delete that line from the repo....but the error I got was the above
There was a problem hiding this comment.
Yes okay I see.
So yep that's a good starting point for your investigations. Good luck with this, really appreciated. Let us know how you go. Cheers
There was a problem hiding this comment.
do you know how to fix the issue with automate_training test with the error above?
There was a problem hiding this comment.
@cakester If I may suggest, running the test within the debugger might give you clues as to where the problem is. Moving forward, running the debugging will also give you more insights about the functional/integration aspects of ivadomed (ie: "big picture" of the project)
cakester
changed the title
cakester
force-pushed
the
hm/NumpyToTensor
branch
from
2678b4d
to
12b1d14
Compare
|
I ran locally everything passes, now you just have to remove NumpyToTensor from all of the configs or else it would fail |
Sounds like a good plan! Thanks!! |
|
someone approve this ivadomed/data_functional_testing#7 |
|
now we just have to wait until https://github.com/ivadomed/data_functional_testing is archived again... |
can you please elaborate what needs to be done? |
right now download_data script is downloading the archived file from the data_functional_testing repo, which is last updated two month ago...after the new commit goes in we have to archive the new data_functional_testing repo and change the download_data script to download the new archived data |
sorry i'm not sure i fully understand-- can you please let us know what needs to be done? do you need any help? |
this is the download_data script Now I just need data_functional_testing to be archived again, then I can change this url to download the latest data_functional_testing, thanks |
is that something you can do? sorry i have limited time |
|
now everything passes, can I get an approval? |
* call NumpyToTensor before NormalizeInstance * fixing tests * call NumpyToTensor last * fix tests * use new release of data_functional_test
Checklist
GitHub
PR contents
Description
note I cannot provide a test for this since ivadomed still fails for the tutorial dataset (which is not the right dataset) for a different reason, we just got past the point where this was failing in the ticket, if you provide me with a dataset that would pass I would be glad to add some tests.
we would have to get rid of all NumpyToTensor in the config or it would be called twice if there is a NormalizeInstance
Linked issues
resolves #815