[Clam-devel] Re: adding the documents to the scheme of SilenceDetector

[David García Garzón]

On Divendres 06 Juny 2008, JunJun wrote:
> Hi, here I send a patch to populate a pool, and populate segmentation with
> random data.

i like the solution you did for the data, using an external function to fill 
it, but cut & paste is bad. What is doing a CLAM::Segment in there? I assure 
you it was harder adding a comment and the include directive than just 
removing the parameter. CLAM::Segment is a legacy monster, so don't couple 
your code to it if there is no need to.

> I think sending patches might be better than documentation of the
> exploration. 

Patches just make us happy.

> The patch should get the full path within the repository. Please take a
> look at the diffs. 

Solve at least the tabulators question and it will go in. The Segment thing 
could be nice if it is included in the tabulators patch.

> The memory of Windows_mingw native compile starts to 
> fade.

I think we should split the processing lib so that the process is not that 
memory hungry. Besides the linking of libclam_processing is there any other 
critical point your computer does not respond?

> Could you drop some more patch-specific targets on the Todo 
> list. Is it my task to  update the wiki? 

Yes, TODO wiki is yours. I can give you input on how to proceed but i would 
prefer you to organize them as your will.

I also like the entry in your blog as to document your research. Nice.

I propose you the following exercise that will step forward on integrating 
such duplicated worlds, will give you more confidence on modifying others 
code.

The proposal is adding a method to each segmentation that fills a ref passed 
DataArray properly, so that at least the mapping is controlled in a place 
where you can reuse it, for example in you extractor. We currently have the 
second constructor to do one of the ways i am proposing the way back.

Note that the key in this exercise, apart on getting insight in inner code, is 
that you realize on how to do small steps to change existing code without 
breaking it (doing unit test it would be a refactoring[1]). It is very 
important to choose the steps in such order that the code is always working. 
Send a patch on every step.

[1] http://www.refactoring.com/

So the steps, in non-breaking-code-each-step-order, would be:
- Adding a virtual method fillArray(DataArray&) on Segmentation with an empty 
implementation
- Choosing one of the segmentations, and adding the method in there also empty
- Copy (dont move!) the code in SegmentationPane::updateSegmentation for the 
kind of segmentation, into the new method, adapted to the new context
- Change the code in the switch with a call to the method
- Test that this kind is initialized properly
- Proceed the same with the other two kinds of segmentation (notice that one 
kind is not implemented yet)
- Once every branch in the switch contains just a call to the virtual 
function, use virtuallity instead the switch. (note that the missing kind 
will work the same since for such kind we are creating a DiscontinuousSeg in 
refreshSegmentation)
- Then remove any unused variable left in updateSegmentations. You should end 
up with a 6 lines method vs 39!

Once you have moved this to the Segmetation classes you could consider using 
it in your extractor.

The next step should be making the segmentation XML aware and getting ride of 
the DataArray for segmentations, and, at the same time, removing a bug we 
detected in a past session. We can do that in remote pair programming, so 
when you are almost done on those steps above let's schedule a session this 
weekend for the XML.

David.


> ----- Original Message -----
> From: "David García Garzón" <dgarcia at iua.upf.edu>
> To: "JunJun" <wangjun at dsp.ac.cn>
> Cc: <clam-devel at llistes.projectes.lafarga.org>
> Sent: Friday, May 30, 2008 4:22 PM
> Subject: Re: adding the documents to the scheme of SilenceDetector
>
> > Be careful when merging. You almost reverted the changes i did: changing
> > the uri before adding any attribute and using literals to set the
> > documentation instead intermediate vars.
> >
> > El Friday 30 May 2008 04:17:52 JunJun va escriure:
> >> Hi!
> >>
> >> >BTW, you may end up editing files in CLAM/src so it's better if you
> >> > generate the patches from an upper level to comprise all the
> >> > repository.
> >>
> >> no, I edited files in mingw/local/include/CLAM, which have no svn
> >> function, what to do?  The changes there, however, are all for the
> >> native debug and are meaningless to patch them.
> >
> > Anyway do the patches from the bottom of the clam repository. Still you
> > can restrict the changes included in the path specifying the files or
> > directories on the command line, for example:
> >
> > svn di Annotator/src CLAM/src/data/Description/Pool/Schema.hxx > my.patch
> >
> > The important bit is that you get the full path within the repository on
> > the patch.
> >
> >> I 'd like to add a TODO to the session list. Please check it out-
> >
> > Although i am adding thing  to the TODO's, it is you tool so feel free to
> > use it.
> >
> >> TODO: fix one of the pool's inconsistent states- Given a schema, what
> >> should pool do with no added scope instances? * Given a description
> >> scheme, and given a user-defined descriptors file suffix (.detector), an
> >> empty pool should be dumped. e.g. given the schema of SilenceDetector:
> >>            <?xml version="1.0" encoding="UTF-8"?>
> >>            <Schema>
> >>
> >> <Uri>descriptionScheme:www.iua.upf.edu:clam:JunJunSilenceDetector</Uri>
> >> <Attributes>
> >>                <Attribute name="Silences" scope="Song"
> >> type="Segmentation"> <ChildScope></ChildScope>
> >>                    
> >> <SegmentationPolicy>Discontinuous</SegmentationPolicy> </Attribute>
> >>             </Attributes>
> >>           </Schema>
> >>
> >> the empty pool named "x.mp3.detector" would be like:
> >> <?xml version="1.0" encoding="UTF-8"?>
> >> <DescriptorsPool>
> >>   <ScopePool name="Song" size="1">
> >>     <AttributePool name="Silence" size="0"/>
> >>   </ScopePool>
> >> </DescriptorsPool>
> >>
> >> One problem is, how to initiate the size of the TBD AttributePool  ,
> >> size="0"? size="1"?
> >
> > I guess that the size attribute of the AttributePool refers the size of
> > the one and only segmentation. You can add segmentations and it will
> > increase. But you spotted another 'bug', congratulations! If you had two
> > songs, as the Attribute pool can have just a single size attribute, just
> > the last one would survive. So we need an inner XML element for each
> > segmentation. 'Gladly' we have used segmentations just on the song level
> > and normally Song is a singleton scope, so we can keep it like this by
> > now but write down the bug on the TODO's.
> >
> > I added some steps to spot even more irregularities.
> >
> > David.



-- 
David García Garzón
(Work) dgarcia at iua dot upf anotherdot es
http://www.iua.upf.edu/~dgarcia
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.clam-project.org/pipermail/clam-devel-clam-project.org/attachments/20080606/2bc60345/attachment.sig>