populse / mia_processes

The default processes repository for mia
Other
1 stars 2 forks source link

[tags inheritance] spm stat bricks #39

Closed manuegrx closed 1 year ago

manuegrx commented 1 year ago

In EstimateModel and EstimateContrast we need to add a "Contrats num" tag.

This tag is added using set_dbFieldValue :

 self.inheritance_dict[value] = self.spm_mat_file
  if key == "out_spm_mat_file" and self.factor_info:
      # Add tag for number of contrast created in database
      # FIXME: if brick use in a pipeline, this tag is
      # not kept in the database
      # (but the pipeline is working)
      tag_to_add = dict()
      tag_to_add["name"] = "Contrasts num"
      tag_to_add["field_type"] = FIELD_TYPE_INTEGER
      tag_to_add["description"] = "Total number of contrasts"
      tag_to_add["visibility"] = True
      tag_to_add["origin"] = TAG_ORIGIN_USER
      tag_to_add["unit"] = None
      tag_to_add["default_value"] = None
      tag_to_add["value"] = (
          number_t_contrast + number_f_contrast
      )
      set_dbFieldValue(self.project, value, tag_to_add)

When we are using each brick, the tag is correctly added to the database:

image

But when we are using those bricks in a pipeline, the tag is set to Not Defined in the database (however during the pipeline we can acess to the tag using get_dbFieldValuefunction )

Two issues :

servoz commented 1 year ago

Can you give please the minimum steps to reproduce the issue?

manuegrx commented 1 year ago

How to reproduce ? I am sorry I do not know how to reproduce this issues simply. I propose to use data used for the closed issue https://github.com/populse/mia_processes/issues/47 (if you still have the data ! )

  1. Add the data (swrafunc_all.nii and a rp file ) in a project

  2. Use this pipeline and fill "sess_scans" with swrafunc_all.nii and "sess_multi_reg" with the realignment file. This pipeline combine level1Design, estimate model and estimate contrast brick and all the parameters are defined for this subject. In this case, at the end of the run, the "Contrats num" tag is empty for all the data. However, during the initialization this tag is correctly set as the pipeline running without issue. I do not know with at the end in the database the tag is empty

  3. Remove all the document created during the previous pipeline:

I think there is one issue :

And one question :

servoz commented 1 year ago

OK, I reproduce, for the whole pipeline (step 1 to 2). I'm going to start working on this ticket by trying to index the tag value in the database (I'll then look at how to keep the tag only for the documents we want).

servoz commented 1 year ago

This ticket is strongly linked to the populse/populse_mia#290 ticket and depends more on populse_mia than mia_processes.

In summary, the indexing of the database to enable the inheritance of the output documents of a brick from the tags of an input document of this brick is only carried out in V2 of Mia at the end of initialisation. This approach is elegant and relies on the workflow object instantiated during initialisation.

However, this approach represents a major regression compared with V1, namely that it is no longer possible to use a tag, in a brick, added or modified in a previous brick, in the pipeline during initialisation. This considerably reduces the interest in having an ubiquitous database in Mia. As a quick dirty patch, in order not to rewrite what should be a substantial part of the PipelineManagerTag.init_pipeline() method, we decided to add the necessary tags later directly to the bricks using the mia_processes.utils.set_dbFieldValue() method ... quick and dirty ...

This works perfectly (even if it's a bit tricky to code because you have to add the necessary tags to the bricks one by one) in most cases (for example, if the tag exists at the beginning of the brick, in this case the tag added during initialisation will be added again at the end of initialisation, which doesn't change anything in the end). Here we reach the limit of the patch ... The tag does not exist as input, so it is added during initialisation, and at the end of initialisation inheritance is carried out, but as the tag does not exist as input ... the value is deleted.

It's maybe possible to add a patch to the patch ... we're going to end up with a labyrinthine system. ... or maybe it's time to cleanly modify the populse_mia machinery to allow tags to be inherited at the end of each brick's initialisation rather than at the end of the entire initialisation. I'm not sure what the best solution is. On the one hand, it may be the time to make a clean change in populse_mia, on the other hand, given the number of things we have to do, we like quick fixes.... I think I'll have a quick look at whether a new patch is possible and also try to assess the difficulty of making a clean change to populse_mia ...

I'm on vacation tonight. Except improbable good surprise I doubt to have time to propose a fix before the beginning of August!

servoz commented 1 year ago

Concerning the first part of the ticket (empty tag): I've made a small change to the EstimateContrast and EstimateModel bricks (issue#39 branch). @manuegrx, can you please run the tests you think are necessary to validate the fix?

In fact, for this first part, there is no real problem, a priori. Just remember that:

1- all the tags in foo1 are inherited into foo2 using: self.inheritance_dict[foo2] = foo1

2- inherit all the tags from foo1 into foo2, but add/change a tag (for example the Contrasts num tag which has the value 10) with:

self.inheritance_dict[foo2] = dict()
self.inheritance_dict[foo2]["parent"] = foo1
self.inheritance_dict[foo2]["own_tags"] = []
# Add tag for number of contrast created in database
 tag_to_add = dict()
 tag_to_add["name"] = "Contrasts num" ("number of contrasts")
 tag_to_add["field_type"] = FIELD_TYPE_INTEGER
 tag_to_add["description"] = "Total number of contrasts"
 tag_to_add["visibility"] = True
 tag_to_add["origin"] = TAG_ORIGIN_USER
 tag_to_add["unit"] = None
 tag_to_add["default_value"] = None
 tag_to_add["value"] = 10
 self.inheritance_dict[foo2]["own_tags"].append(tag_to_add)

I actually made a mistake in my previous post. The first part of this ticket is not linked to the populse/populse_mia#290 ticket (phew!).

Concerning the second part of the ticket, Given that we inherit the con_images output , ess_images output, etc. from SPM.mat and that we add the Contrasts num tag to SPM.mat, it is difficult not to have this tag in con_images, ess_images ... As we discussed privately with @manuegrx , there should be an option to remove a tag at the time of inheritance. I'm looking at what we can do in this direction.

@manuegrx as soon as you validate the fix I'll merge it into master.

manuegrx commented 1 year ago

@servoz test done, it is working ! Thank you, you can merge the fix into master !

servoz commented 1 year ago

Thanks @manuegrx , issue#39 branch is now merged in master. I'm going to open a ticket in populse_mia for the possibility of deleting a tag from a document during the inheritance phase. In this way, we'll be able to

I think we can close now this ticket.