Developer Meetings/20120508

Attendees: Alex, Christopher, Greg, Jc, Julien, Nicole, Steve, XioJun

• Welcome to Christopher Mullins - Kitware intern from UNC Commputer Science that will be working on Slicer

To discuss

• Road map - http://www.na-mic.org/Bug/roadmap_page.php
  • Concept of milestone with target date
  • Issues associated with a milestone
  • Ideally, all work having the form of a commit should be represented by a Mantis issue.

• Release schedule - Release Often, release better :)
  • Step1: Transition to git -> master / next
  • Step2: Plan of quarterly release
  • Concept of submaintainers: DICOM, CTK, VS2010 Support, VTK, Python support, Annotation, MRML, ...

A lot of those contributions come from people who make just tiny one-liner changes, and some of them are never heard from
 again once they got their one small fix done, but on the other hand, the small one-liner changes is how many others gets started.

Most of those one-liners didn’t get to me directly – many of them came through [...] submaintainers etc. 

We have a very strict “no regressions” rule, for example, and a large part of that rule is so that people – very much including the people
involved in distributions – don’t need to fear upgrades. If it used to work a certain way, we try very hard to make sure it continues 
to work that way. Sure, bugs happen, and some change may not be noticed in time, but on the whole I think a big part of kernel 
development is to try to make it as painless as possible for people to upgrade smoothly.

Because if you make upgrades painful, it just means that people will stay back.

Source: http://techcrunch.com/2012/04/19/an-interview-with-millenium-technology-prize-finalist-linus-torvalds/

• Problem with the SceneView nodes
  • http://www.na-mic.org/Bug/view.php?id=1993
  • http://www.na-mic.org/Bug/view.php?id=1995
  • Commit a failing test ?

Problem with the SceneView nodes

Hi Alex,

I've been working on the issue, and I'll commit a fix soon.
While investigating what was going wrong, I ended up doing some interesting discoveries:
 - vtkMRMLDisplayableNode behavior was inconsistent. I fixed it in a series of commits this past week.
 - vtkMRMLSceneViewNode::RestoreScene() is doing something dangerous. 
I added a unit test to reveal the problem:http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=20030.
This mostly results in "random" crashes when restoring multiple scenes. I created a Mantis issue: http://www.na-mic.org/Bug/view.php?id=1993.

To summarize, the problem is that RestoreScene (https://github.com/Slicer/Slicer/blob/master/Libs/MRML/Core/vtkMRMLSceneViewNode.cxx#L472), is adding its internal nodes into the scene, if the node is modified, then restoring the scene again will restore the modified version instead of the original version. 

I believe a fix would be to create a copy of the node and add the copy into the scene instead of the node itself. As it is a sensitive change, lots of tests must be done. 

Alex: do you have time to look at it ? 

All: I added a test into slicer that will be failing until the issue is fixed. I wonder if it's the correct process we want to follow.

Thanks,
Julien.

Hi Ron & Alex, 

r20033 fixes the original issue: restoring the sceneview CST-R from  2012-04-27-aDemo-PeritumoralTractParcellation  doesn't crash anymore.

However, r20034 identifies more issues with the scene views. I created a Mantis issue: http://www.na-mic.org/Bug/view.php?id=1995 in addition to the one I created earlier http://www.na-mic.org/Bug/view.php?id=1993.

All:
As a personal side note, the current Node/NodeID/ReferenceID/UpdateScene/... MRML mechanism is extremely complex.
It is humanly very challenging to have a clear understanding of the whole system (this explains the unlimited source of bugs in MRML :-) ). 
We should seriously rethink the MRML scenes and observations to have a unique way to "link" nodes together. 
I guess it's another topic for the summer project week :-)

Conclusion

• Validated the concept of roadmap. Not all issues will be associated with targets. Nevertheless, issues people are working on should be associated with a target.

• Generally, when a given developer wants to demonstrate a bug but doesn't have the expertise to fix the bug, it's okay to push failing to test to illustrate the problem. For example: See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=20030

• Julien lead the discussion about SceneViews issues, now Alex has a better idea of the problem and will be working on fixing the problem relying on the failing test Julien pushed.

• More tests is good to prevent regression.

• Internally MRML scene uses Reference whereas externally Scene event are used, a unified mechanism could probably be implemented.

• Some utility functions allowing to easily couple MRML node together will make everything simpler. The idea is that Alex, Steve and Julien will start to write down a list of requirement / specification.