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
[Todo for V2] Reconnecting the iteration #186
Comments
Databasing needs a complete check / rework (independently of iterations), that's sure (and discussed in #181).
Thus the whole chain of operations to build the iteration pipeline is not straightforward, and a real work is needed to make it user friendly, but once the pipeline is built, it works for the user just like a regular pipeline. I dare to say that the former way of using iterations (in v1) was not really easier and intuitive to setup. |
To me the "double filtering", one in the iteration GUI, and the other through "input filers" is strange, non-intuitive, and difficult to use for an inexperienced user who has to select filtering at several places. The iteration GUI is not very clear to me: it splits the whole database according to a given tag values, and produces lists of lists of scans. But I don't understand how/where they are used. The The "Filter" button in the iteration GUI opens a dialog which should be entirely redesigned: all tag values are displayed in a single page, with checkbox buttons for each value. Every single one has to be clicked to actually select/unselect values. In our databases we have thousands, sometimes tenths of thousands, and soon hundreds of thousands of subjects: they don't fit in a single page, and it's not possible to select every one by hand. |
To improve the GUI and simplify user experience, maybe we could do this:
|
The improvements above have been implemented. |
I just started again to try to do something with the iteration, but I crash mia in less than 30s ... Please, can you give a minimum step to run, say, mia_processes.pipelines.preprocessing.Spatial_preprocessing_1, with 3 patients? |
I admit I didn't try this pipeline after reworking the iteration GUI, and there are a few bugs left that show up on it. I'm looking at it. |
I have a problem when iterating a pipeline without exporting its plugs, in this case all nodes inside the iterated pipeline get disabled (the pipeline inside the iteration is actually a pipeline, with everything connected, but all nodes disabled). I have to find what is going on. When exporting the plugs before iterating, things seem to go better (I have not run it yet), but there is something strange there. I'm investigating... |
when taking a pipeline out of itsparent pipeline (#186)
There are also parameters setting problems in nipype nodes (or Mia processes containing nipype nodes, I don't know) - some parameters get the same value for all iterations, which is wrong. |
I don't understand. |
If you iterate on a process, each parameter can become a list, so that you can use either the same value for all iterations, or a different value for each. This is true for all parameters (input images, output files, numeric parameters etc). |
This was causing notifications firing for deleted widgets, and throwing exceptions which caused trouble in parameters completion (#186)
Found a way of doing it via the destroyed signal (#186)
I have reworked several problems showing up in iterations (not all directly linked to iterations), in the MIA parameters completion engine, in the node controllers which were not correctly destroyed and were firing notifications long after they were dead, in populse_mia, in capsul and in soma-base. I also had problems of parameters checking which were different depending on nipype versions. I have pushed a "more stable" version now which seems to work for me on 2 different machines installed differently (one in a containerized build, the other direclty on the system). |
I finally have some time to test the new iteration. Starting with a new, empty pipeline tab in the Pipeline Manager:
|
Ok I succeeded using "mia_processes > bricks > tools> Files_To_List" brick instead of "capsul > pipeline > custom_nodes > reduce_node > ReduceNode" brick (it may be that the problem is caused because the default value of ReduceNode is [''] and func_files expects either a list of existing paths or []?) |
Ok, after the run I get all the expected results in the derived_data directory but as expected only the last patient of the iteration is indexed in the database. So this is a great advance, the direct mode iteration seems to work !! It remains to be seen:
After these two remarks, I find this mode very interesting. The only reservation I see is the possibility of an easy error if we have a lot of data to manage because the lists must be in the same patient order. |
ReduceNode: The database issue is more difficult and will probably need some work: in MIA process runs are indexed using a process UUID which is associated with a process instance, not to a "run". In iterations the same instance will be reused several times (for each iteration), so we have to make changes in this infrastructure. So yes it deserves a separate ticket. |
Oh Yeah !!!! |
I'm still testing:
|
I'm still testing: The documentation for this part is a bit more complex to follow and may be partly obsolete (there are several links, some of which seem to correspond to the V1 iteration), which may lead to confusion. I will rewrite this part when I have managed to run this mode, which at first sight is the closest to the iteration in V1 (which had the advantage for the user not to have to worry about the order of the patients). As I had a bit of trouble understanding the documentation for this part (maybe a leftover influence from the V1 iteration method?), maybe I did it wrong, but the initialisation fails and I observe the following exceptions:
So, there is a traits.trait_errors.TraitError issue. |
Well I don't know how you have built your pipeline here, but I agree the doc for this use case is not clear enough since it actually groups several cases. |
Iteration Via input filters - Manually OK, I did it, but the documentation is obsolete and it is not worth following for this part. I will rewrite it.
There are certainly still some issues to be solved. For example after the initialisation if we go to see the data browser
It looks like a bug. I think we can open a ticket for that (loss of the run button and loss of all indexing if a new initialisation is done, in fact this makes 2 tickets ..) |
In order to close this ticket and to propose a new documentation as clear as possible, for the iteration part, I start again to test from the beginning the different possibilities of use.
Stdout:
|
Ok I think the iteration produces a race condition for the creation of directories. I will propose ASAP a small PR in capsul to fix this issue. |
I have just reproduced exactly the same minimum steps that produced a run fall in the previous post and this time the run worked perfectly... It seems that the problem occurs erratically and it could be a race condition. |
We can therefore conclude that the iteration works in Direct Iteration mode (current documentation).
|
The Via Input_Filter brick (without use of the iteration table) mode:
|
The Via Input_Filter brick (with use of the iteration table) mode:
|
allwos to specify which parameters will be iterated, and which ones will be connected to an input filter and the database. Such nodes are insterted and connected automatically. Lists in iterated processes/pipelines which are connected to an input filter are also added a Reduce node to transform files into lists as they expect.
when taking a pipeline out of itsparent pipeline (#186)
This was causing notifications firing for deleted widgets, and throwing exceptions which caused trouble in parameters completion (#186)
Found a way of doing it via the destroyed signal (#186)
It is pressing need for the clinical application, but also for the research activity, to reconnect the iteration.
At least as it existed in V1 (selection of inputs automatically from the database with filters).
There were imperfections in the V1 iteration (the main one being that there was no initialisation step separated from the run).
The initialisation step, as it currently exits, could be made to disappear.
Indeed, at present, it make inappropriate operations (e.g. outputs are entered into the database when they do not already exist, etc.) in addition to verifying that the run step should run without trouble.
There is yet a lot of thinking to be done on this initialisation step and decisions to be made on exactly how it will work as it may have impact in the code for iteration (and not only!).
But maybe we are getting out of the scope of this ticket . Getting it back would be nice for now :-)
The text was updated successfully, but these errors were encountered: