> > Hello,
> > 
> > It seems some compliers have a problem with making ROME, sending error message
> > saying,
> > 
> >  In file included from builder/src/ROMEBuilder.cpp:574:
> > /home/pekochan/rome/20050626/rome/include/ROMEString.h:41: type spec ifier
> >    omitted for parameter `va_list'
> >  ...etc
> > 
> > 
> > This problem can be fixed in file "ROMEString.h" by adding the following 
> > either line;
> > 
> > #include <stdarg.h>
> > 
> > or 
> > 
> > #include <Varargs.h>
> 
> 
> Thanks for reporting that problem. 
> 
> #include <stdarg.h>
> is added now in the cvs.
replaced with #include <Varargs.h>

> I think that fUniqueID and fBits in TObject is not useful to store in TTree.
> I found in ROOT users guide that we can disable that with
> MyClass::Class()->IgnoreTObjectStreamer(); 
> .
> 
> If you also think these are useless information, please implement it in ROME.
> 
> 
> 
> second topic.
> This is just a matter of favor.
> We sometimes use TTree interactively. At that time, I feel fRunNumber, fEventNumber, fTimeStamp, 
> fSequentialNumber are long to type.
> I prefer shorter without upper case letter.
> If many people agree it, I would like to ask to change them run,event,timestamp,number like MIDAS analyzer.

MyClass::Class()->IgnoreTObjectStreamer(); is implemented.

In Reset method of folders. Support folder members are ignored currently.
Reset method of support folder member had better to be called.

In addition, isModified() should check if the member support folders were modified.

For example

class A : public TObject
{
protected:
   TypeB* B;
   TClonesArray* C;
.
.
public:
   A() {
      B = 0;
      C = 0;
   };
.
.
.
   void Reset() {
      int i;
      fModified = false;
      if(B) B->Reset();
      if(C) for(i=0;i<C->GetEntries();i++) { ((TypeC*)C->At(i))->Reset(); }
   };

   ClassDef(A,1)
};

> Hello,
> 
> It seems some compliers have a problem with making ROME, sending error message
> saying,
> 
>  In file included from builder/src/ROMEBuilder.cpp:574:
> /home/pekochan/rome/20050626/rome/include/ROMEString.h:41: type spec ifier
>    omitted for parameter `va_list'
>  ...etc
> 
> 
> This problem can be fixed in file "ROMEString.h" by adding the following 
> either line;
> 
> #include <stdarg.h>
> 
> or 
> 
> #include <Varargs.h>


Thanks for reporting that problem. 

#include <stdarg.h>
is added now in the cvs.

Hello,

It seems some compliers have a problem with making ROME, sending error message
saying,

 In file included from builder/src/ROMEBuilder.cpp:574:
/home/pekochan/rome/20050626/rome/include/ROMEString.h:41: type spec ifier
   omitted for parameter `va_list'
 ...etc


This problem can be fixed in file "ROMEString.h" by adding the following 
either line;

#include <stdarg.h>

or 

#include <Varargs.h>

I think that fUniqueID and fBits in TObject is not useful to store in TTree.
I found in ROOT users guide that we can disable that with
MyClass::Class()->IgnoreTObjectStreamer(); 
.

If you also think these are useless information, please implement it in ROME.



second topic.
This is just a matter of favor.
We sometimes use TTree interactively. At that time, I feel fRunNumber, fEventNumber, fTimeStamp, 
fSequentialNumber are long to type.
I prefer shorter without upper case letter.
If many people agree it, I would like to ask to change them run,event,timestamp,number like MIDAS analyzer.

This is a proposal on specifying relation of tasks.

Currently, ROME has task hierarchy system. It may be enough.
But a missing thing is that we can not use "||".

So a possibility is like this.

For example, there are two calibration tasks B and D, and a calculation task A requires one of B or D.
B and D dependes on data filling task C.

This case, one writes down the relation in definition file.
<TaskDependenses>
  <TaskDependense taskname="A"/> B || D </TaskDependense>
  <TaskDependense taskname="B"/> C </TaskDependense>
  <TaskDependense taskname="D"/> C && !B </TaskDependense>
</TaskDependeses>

Then, builder analyzes the relation and re-arranges the execution order like,
C->B->D->A

At run time, ROMEEventLoop checks if depending tasks were executed before calling a task. And when 
B or C is executed, it calls A.

This way users can specify complicated relation like ((A && B) || C ) || !D )....

A problem is that you can not express the relation with indent in configuration file and documentation.


This is just a proposal, I'm not sure which is better, TaskHierarcy or TaskDependense.
Personaly, I am not using task controll.

done.
> In ROMEBuilder.cpp, there are some mismatchs of formart and argument of ROMEString::Set or 
> AppendFormatted.
> 
> 1000       format.SetFormatted("   void Set%%s%%%ds(%%-%ds %%s%%%ds) { f%%s%%%ds = %%s; };
> \n",lb,typeLen,lb,lb,lb); 
> 
> 3980             format.SetFormatted("   %%s%%s*%%%ds  Get%%sAt(int index)%%%ds\n",typeLen-
> folderName[i].Length()-scl,0+nameLen-folderName[i].Length(),lt);
> 
> 5875    buffer.AppendFormatted("   bool CheckConfiguration(int runNumber);\n",shortCut.Data()); 
> 
> 6487    buffer.AppendFormatted("#include <TBranchElement.h>\n",shortCut.Data()); 
> 
> 7231       buffer.AppendFormatted("   return buffer;\n",stringBuffer.Data());
> 
> 7271                buffer.AppendFormatted("   if (!this->GetDataBase(name.Data())->Read
> (values,path,gAnalyzer->GetCurrentRunNumber())) {\n",folderName[i].Data(),valueName[i][j].Data());
> 
> 7358          buffer.AppendFormatted("%s                  xml->WriteElement(\"HistArrayStartIndex\",\"0\");
> \n",blank.Data(),pointerI.Data(),histoName[taskHierarchyClassIndex[i]][j].Data()); 
> 
> 8771    buffer.AppendFormatted(" obj/%sAnalyzer.obj obj/%sEventLoop.obj obj/%sConfig.obj obj/
> main.obj",shortCut.Data(),shortCut.Data(),shortCut.Data(),shortCut.Data(),shortCut.Data());
> 
> 8815    buffer.AppendFormatted("     g++ $(Flags) -o $@ $(objects) $(Libraries)\n\n",shortCut.Data
> (),mainProgName.Data());
> 
> 8872    buffer.AppendFormatted("obj/%sFAnalyzer.obj: src/framework/%sFAnalyzer.f src/framework/%
> sAnalyzer.cpp include/framework/%sAnalyzer.h\n",shortCut.Data(),shortCut.Data(),shortCut.Data());
> 
> 9341    htmlFile.SetFormatted("%s%sUserHTML.html",outDir.Data(),shortCut.Data(),mainProgName.Data
> ());

done.

> About identical check of new and existing file in builder.
> 
> Currently there is a limit of file size defined by "bufferLength".
> When the file size exceed the size, the file will be always overwritten by builder.
> And if the file is existing task cpp file, user will lose a part of his code.
> 
> Actually, LPConfig.cpp is larger than that.
> 
> So I wrote a simple function. it will be called recursively when the file is larger than buffer.
> bufferLength is not necessary to be so large.
> 
> bool checkIdentical(const int fileHandle,const char* pattern,bool flag=true)
> {
>    static int position
>    int  i;
>    char fileBuffer[bufferLength];
> 
>    if(flag)
>       position = 0;
> 
>    int nb = read(fileHandle,&fileBuffer, sizeof(fileBuffer));
>   
>    if(nb == bufferLength){
>       if((int)strlen(pattern)-position < bufferLength)
>          return false;
>      
>       for (i=0;i<nb;i++)
>          if (pattern[position+i] != fileBuffer[i])
>             return false;     
>       position += bufferLength;
>       return checkIdentical(fileHandle,pattern,false);
>    }  
>    else{
>       if (nb != (int)strlen(pattern) - position)
>          return false;
>      
>       for (i=0;i<nb;i++)
>          if (pattern[position+i] != fileBuffer[i])
>             return false;
>    }
>    return true;
> }
> 
> 
> You may use this function in builder like,
>    int fileHandle = open("test.txt",O_RDONLY);
>    bool identical = checkIdentical(fileHandle,buffer);
>    close(fileHandle);
> 
> And there is another way. TString can read file. this featrue can be used when you read existing Task cpp file.
> 
>    ifstream ifile("test.txt");
>    TString fileBuffer;
>    str.ReadFile(ifile);
>    bool identical = ( fileBuffer == buffer );

In ROMEBuilder.cpp, there are some mismatchs of formart and argument of ROMEString::Set or 
AppendFormatted.

1000       format.SetFormatted("   void Set%%s%%%ds(%%-%ds %%s%%%ds) { f%%s%%%ds = %%s; };
\n",lb,typeLen,lb,lb,lb); 

3980             format.SetFormatted("   %%s%%s*%%%ds  Get%%sAt(int index)%%%ds\n",typeLen-
folderName[i].Length()-scl,0+nameLen-folderName[i].Length(),lt);

5875    buffer.AppendFormatted("   bool CheckConfiguration(int runNumber);\n",shortCut.Data()); 

6487    buffer.AppendFormatted("#include <TBranchElement.h>\n",shortCut.Data()); 

7231       buffer.AppendFormatted("   return buffer;\n",stringBuffer.Data());

7271                buffer.AppendFormatted("   if (!this->GetDataBase(name.Data())->Read
(values,path,gAnalyzer->GetCurrentRunNumber())) {\n",folderName[i].Data(),valueName[i][j].Data());

7358          buffer.AppendFormatted("%s                  xml->WriteElement(\"HistArrayStartIndex\",\"0\");
\n",blank.Data(),pointerI.Data(),histoName[taskHierarchyClassIndex[i]][j].Data()); 

8771    buffer.AppendFormatted(" obj/%sAnalyzer.obj obj/%sEventLoop.obj obj/%sConfig.obj obj/
main.obj",shortCut.Data(),shortCut.Data(),shortCut.Data(),shortCut.Data(),shortCut.Data());

8815    buffer.AppendFormatted("     g++ $(Flags) -o $@ $(objects) $(Libraries)\n\n",shortCut.Data
(),mainProgName.Data());

8872    buffer.AppendFormatted("obj/%sFAnalyzer.obj: src/framework/%sFAnalyzer.f src/framework/%
sAnalyzer.cpp include/framework/%sAnalyzer.h\n",shortCut.Data(),shortCut.Data(),shortCut.Data());

9341    htmlFile.SetFormatted("%s%sUserHTML.html",outDir.Data(),shortCut.Data(),mainProgName.Data
());

About identical check of new and existing file in builder.

Currently there is a limit of file size defined by "bufferLength".
When the file size exceed the size, the file will be always overwritten by builder.
And if the file is existing task cpp file, user will lose a part of his code.

Actually, LPConfig.cpp is larger than that.

So I wrote a simple function. it will be called recursively when the file is larger than buffer.
bufferLength is not necessary to be so large.

bool checkIdentical(const int fileHandle,const char* pattern,bool flag=true)
{
   static int position
   int  i;
   char fileBuffer[bufferLength];

   if(flag)
      position = 0;

   int nb = read(fileHandle,&fileBuffer, sizeof(fileBuffer));
  
   if(nb == bufferLength){
      if((int)strlen(pattern)-position < bufferLength)
         return false;
     
      for (i=0;i<nb;i++)
         if (pattern[position+i] != fileBuffer[i])
            return false;     
      position += bufferLength;
      return checkIdentical(fileHandle,pattern,false);
   }  
   else{
      if (nb != (int)strlen(pattern) - position)
         return false;
     
      for (i=0;i<nb;i++)
         if (pattern[position+i] != fileBuffer[i])
            return false;
   }
   return true;
}


You may use this function in builder like,
   int fileHandle = open("test.txt",O_RDONLY);
   bool identical = checkIdentical(fileHandle,buffer);
   close(fileHandle);

And there is another way. TString can read file. this featrue can be used when you read existing Task cpp file.

   ifstream ifile("test.txt");
   TString fileBuffer;
   str.ReadFile(ifile);
   bool identical = ( fileBuffer == buffer );

> > As ROOTSYS is required for the ROME build, I would suggest to
> > include the code below in the builder for the application Makefile.
> > This will make Rome less dependent on the ROOT path.
> > 
> > Replace in ROMEBuilder.cpp:
> > 
> > buffer.AppendFormatted("rootcint -f %sDict.cpp -c -p ",shortCut.Data());
> > 
> > by
> > 
> > buffer.AppendFormatted("LD_LIBRARY_PATH=$(ROOTSYS)/lib $(ROOTSYS)/bin/rootcint
> > -f %sDict.cpp -c -p ",shortCut.Data());
> 
> That's in the cvs now, thanks.

LD_LIBRARY_PATH is not dedecated for ROOT. One can already have it, and overwriting it may cause error.
For instance, if he installed libstdc++ in non-standard place, he will set LD_LIBRARY_PATH there.

In case of GNU Makefile, we can use "if" statemente like,
ifdef LD_LIBRARY_PATH
LD_LIBRARY_PATH += $(ROOTSYS)/lib
else
LD_LIBRARY_PATH := $(ROOTSYS)/lib
endif

> 
> > Then user can change the variable with command line option like
> > ./megframework -m -t 2;
> > 
> > Then builder creates new method.
> > (Bool_t) MEGAnalyzer::GetisMC()
> > (Int_t) MEGAnalyzer::GetTriggerType()
> 
> I realized better way. In this way users can add or remove command line options without changing their
> task files. And it is not necessary to add new object in ROME.
> 
> What I propose is to make it in steering parameter. That is <SPCOmmandOption> in steering parameter's
> properties.
> 
>   <SteeringParameterField>
>     <SPFieldName>isMC</SPFieldName>
>     <SPFieldType>Bool_t</SPFieldType>
>     <SPCommandOption>m</SPCommandOption>
>   </SteeringParameterField>
>   <SteeringParameterField>
>     <SPFieldName>TriggerType</SPFieldName>
>     <SPFieldType>Int_t</SPFieldType>
>     <SPCommandOption>t</SPCommandOption>
>   </SteeringParameterField>
> 
> Then user can change the variable with command line option like
> ./megframework -m -t 2;
> when -m is typed, isMC becomes true.
> Trigger type will be 2.
> 
> They can specify the value both with config file and command line option just same as run numbers.
> Values in config file will be overwritten by command line option.
> 
>  

done.

> Then user can change the variable with command line option like
> ./megframework -m -t 2;
> 
> Then builder creates new method.
> (Bool_t) MEGAnalyzer::GetisMC()
> (Int_t) MEGAnalyzer::GetTriggerType()

I realized better way. In this way users can add or remove command line options without changing their
task files. And it is not necessary to add new object in ROME.

What I propose is to make it in steering parameter. That is <SPCOmmandOption> in steering parameter's
properties.

  <SteeringParameterField>
    <SPFieldName>isMC</SPFieldName>
    <SPFieldType>Bool_t</SPFieldType>
    <SPCommandOption>m</SPCommandOption>
  </SteeringParameterField>
  <SteeringParameterField>
    <SPFieldName>TriggerType</SPFieldName>
    <SPFieldType>Int_t</SPFieldType>
    <SPCommandOption>t</SPCommandOption>
  </SteeringParameterField>

Then user can change the variable with command line option like
./megframework -m -t 2;
when -m is typed, isMC becomes true.
Trigger type will be 2.

They can specify the value both with config file and command line option just same as run numbers.
Values in config file will be overwritten by command line option.

 

> As ROOTSYS is required for the ROME build, I would suggest to
> include the code below in the builder for the application Makefile.
> This will make Rome less dependent on the ROOT path.
> 
> Replace in ROMEBuilder.cpp:
> 
> buffer.AppendFormatted("rootcint -f %sDict.cpp -c -p ",shortCut.Data());
> 
> by
> 
> buffer.AppendFormatted("LD_LIBRARY_PATH=$(ROOTSYS)/lib $(ROOTSYS)/bin/rootcint
> -f %sDict.cpp -c -p ",shortCut.Data());

That's in the cvs now, thanks.

As ROOTSYS is required for the ROME build, I would suggest to
include the code below in the builder for the application Makefile.
This will make Rome less dependent on the ROOT path.

Replace in ROMEBuilder.cpp:

buffer.AppendFormatted("rootcint -f %sDict.cpp -c -p ",shortCut.Data());

by

buffer.AppendFormatted("LD_LIBRARY_PATH=$(ROOTSYS)/lib $(ROOTSYS)/bin/rootcint
-f %sDict.cpp -c -p ",shortCut.Data());

<Online> in configuration file is MIDAS specific thing. Non MIDAS users don't need it.
And it can be used when MIDAS is linked.

So it is better that <Online> appears only when MIDAS is linked.

At first there must be several opinions and discussions. Following is just my opinion.
Probably asking opinion some users before changing ROME is better.

There are some properties for histograms and databases.

* database name
* database type and connection
* field database name and path
* histogram properties

I assume that definition file is shared by several persons in experimental group and configuration files 
are created for each users at the first execution. If you assume that we should share also configuration 
file, story written below is different.

My basic concept is that
 *Parameters which is complicated or will be shared among several users should be in definition file.
 *Parameters which can be often changed at run time should be in configuration file.

The good point to specify values in definition file is that they are flexible, because we can use 
ROMEString::SetFormatted.
It is outstanding point of ROME that we can design framework very flexibly due to builder.

Other point is that parameters in definition file will be written by programer while those in 
configuration file can be written by non-experts. It is better that the application can work with few 
changes of configuration file as possible. In my feeling, to specify database paths and histogram 
parameters in configuration file correctly is too tough for normal users.

In my opinion, they are separated like,
Definition file:
* field database name and path
* histogram properties

Configuration file:
* database type and connection
* (histogram properties)

-- about database path
Normally, users share the same database structure. So I think it should be in definition file. I don't think 
paths are changed so often.

-- about database type and connection
At first type and connection can be written in one line like,
   xml://directory_of_database/database_name
   mysql://[user_name]:[passwd]@[hostname]:[port]/database_name
   none://
      (  odb://[host_name]/[experiment] or odb://experiment@host_name  )
(parameters in [] are optional)

These parameters will be changed user by user, because they can have xml files in different place, and 
they can use replicated SQL servers.
Password can be written in connection. So it should be in configuration file.

-- about histogram parameters
We can specify them with calculation if they are in definition file. I think it should be in definition file.
And to allow to overwrite in configuration file is also attractive. In my opinion overwriting with normal 
type like Float_t,Int_t... is enough.

---------
In result, my opinion is like following.
Database name, database paths and histogram parameters are in definition file.
In default configuration file, config class writes database entries appeared in definition file.
Config class writes histogram parameters entries with empty values if the histogram is <Configurable>. 
Only when 
some value is written there, analyzer overwrite these parameters.

As I wrote above I think normal number like following are enough as values of histogram parameters in 
configuration file.
   <HistXmax>1000</HistXmax>
I don't think following is necessary.
   </HistXmax>/GSP/parameter</HistXmax>


Definition file
    <Folder>
        <FolderName>foldername</FolderName>
        <DataBaseAccess>true</DataBaseAccess>
        <Field>
          <FieldName>fieldname</FieldName>
          <FieldType>Float_t</FieldType>
          <DataBaseName>first_database</DataBaseName>
          <DataBasePath>"/Path/to/record"</DataBasePath>
        </Field>
       <Field>
          <FieldName>fieldname2</FieldName>
          <FieldType>Float_t</FieldType>
          <DataBaseName>second_database</DataBaseName>
          <DataBasePath>"/Path/to/record2"</DataBasePath>
        </Field>
   </Folder>
...
     <Histogram>
        <HistName>histname</HistName>
        <Configurable>true</Configurable>
        <HistType>TH1F</HistType>
        <HistTitle>ADC0 Bank</HistTitle>
        <HistFolderTitle>ADC</HistFolderTitle>
        <HistArraySize>gAnalyzer->GetGSP()->GetNumberOfADC()</HistArraySize>
        <HistXNbins>4094</HistXNbins>
        <HistXmin>1</HistXmin>
        <HistXmax>4095</HistXmax>
      </Histogram>

Default configuration file
    <DataBases>
      <DataBase>
        <Name>first_database</Name>
        <Connection>none://</Connection>
      </DataBase>
     <DataBase>
        <Name>second_database</Name>
        <Connection>none://</Connection>
      </DataBase>
    </DataBases> 
...
     <Histogram>
        <HistName>histname</HistName>
        <HistFolderTitle></HistFolderTitle>
        <HistArraySize></HistArraySize>
        <HistXNbins></HistXNbins>
        <HistXmin></HistXmin>
        <HistXmax></HistXmax>
      </Histogram>

>   There is no check if the pointer returned by GetDataBase,GetActiveDAQ is valid. This can result 
> segmentation fault.
>   For instance, if one specifies DAQ system as "none", or one does not specify DataBaseName, rome 
> stops without error message.
> 
> It is better to modify this behavior.
> 
> 
> * One way is to stop application like.
> ROMEDAQSystem* GetActiveDAQ() {
>    if(!fActiveDAQ){
>       gAnalyzer->Println("some error message");
>       fApplication->Terminate(1);
>    }
>    return fActiveDAQ;
> };
> 
> *Other way is to use a dummy DAQSystem and DataBase which does nothing when user does not 
> specify DAQ or DataBase.  When dummy is used, warning message must be shown, because it is 
> dangerous that users do not realize they are using dummy.


done

  There is no check if the pointer returned by GetDataBase,GetActiveDAQ is valid. This can result 
segmentation fault.
  For instance, if one specifies DAQ system as "none", or one does not specify DataBaseName, rome 
stops without error message.

It is better to modify this behavior.


* One way is to stop application like.
ROMEDAQSystem* GetActiveDAQ() {
   if(!fActiveDAQ){
      gAnalyzer->Println("some error message");
      fApplication->Terminate(1);
   }
   return fActiveDAQ;
};

*Other way is to use a dummy DAQSystem and DataBase which does nothing when user does not 
specify DAQ or DataBase.  When dummy is used, warning message must be shown, because it is 
dangerous that users do not realize they are using dummy.