[Clam-devel] Re: Clam-devel Digest, Vol 19, Issue 11

Fri Jun 6 11:03:33 PDT 2008

Half commited. As i told you the code itself is ok.

Regarding the patch, try to be selective on what you put in the patch. The 
same that will happen with commits. If you have several files modified select 
just the files concerning the change you are refering. In this case you have 
added some modifications on the scons that are fully unrelated.

To select a set of files or dirs, run svn-diff providing the files or paths 
like this:
svn diff Annotator CLAM/SConstruct  > my.patch
Those locally changed files should tend to zero so let's talk next time at irc 
how to integrate them in the svn if they are needed in windows.

Also having renamed the Annotator to Annotator_addExtractor has rendered the 
patch unusable without manual tweaking. Your backup source copy is now the 
svn server. No need to copy dirs to keep the old version.

The tabulators issue is not solved. You made it worse by extra indenting those 
lines. The only that you need is to use a single (real) tabulator for each 
indentation level and not using spaces. I'd like not to spend too much time 
with this kind of things, so if you don't understand the point just proceed 
with the exercise i proposed in my previous mail. I'll fix it myself and i 
will explain you at the irc. You'll get a conflict.

Hurray for the monster extermination!!

So right now, follow the steps about the Array filling below the refactoring 
url in my previous mail.

El Friday 06 June 2008 16:33:22 JunJun va escriure:
> Hi David,
> I resend the patch (to populate a pool, and populate segmentation with
> random data).

> I hope I didn't misunderstand the meaning of  "tabulators 
> question". 

> Moreover, the legacy monster is removed. 

> ----- Original Message -----
>
> > 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</Ur
> >> >>i> <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: not available
> > Type: application/pgp-signature
> > Size: 189 bytes
> > Desc: This is a digitally signed message part.
> > Url :
> > http://llistes.projectes.lafarga.cat/pipermail/clam-devel/attachments/200
> >80606/2bc60345/attachment-0001.pgp
> >
> > ------------------------------
> >
> > _______________________________________________
> > Clam-devel mailing list
> > Clam-devel at llistes.projectes.lafarga.org
> > https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel
> >
> >
> > End of Clam-devel Digest, Vol 19, Issue 11
> > ******************************************