>   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

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>

<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.

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());

> 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.

> 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.

 

> 
> > 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.

> > 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

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.

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)
};

> 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.

> 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.
changed to run, event, time and number.

ResetFolders are called in DAQEvent for every events.

I have some folders who's information have to be kept during the run.

One possibility is to add new tag like <ResetBoforeEvents> in folder definition.
The other is give up reset in ROMEEventLoop and give users resposibility of reset.

> ResetFolders are called in DAQEvent for every events.
> 
> I have some folders who's information have to be kept during the run.
> 
> One possibility is to add new tag like <ResetBoforeEvents> in folder definition.
> The other is give up reset in ROMEEventLoop and give users resposibility of reset.

done.

The name of the tag is <NoResetByFramework>

Current file handling of ROME is based on run number. This way is simple and easy to understand. But 
there are some arguments on it.

(i) In case of .mid file. ROME assume the file name is like run00001.mid. But this is not always true. 
MIDAS users can have different name.

(ii) ROME saves one TTree in one file. But there are cases that user want to save several TTrees in a file

(iii) Especially rare event search experiments like MEG, one file may contain signal candidate events 
from several runs.

To solve this issue one idea is to move from run base I/O to filename base I/O.

* Input
Users may specify filename instead of run number with command line option like.
XXXanalyzer.exe -f run00001.mid,run00002.mid

Then ROMEAnalyzer provides information of filename to DAQ class.

How files are handled is depending on DAQ class. Normally each files will be read one by one.
But in case of bartender program, they will be read in parallel to mix.

In case of .mid file, run number is written in run header of files.
ROMEMidas should change current run number at the BeginOfRun.

* Output
For the output filename we can employ same technique as database path.
This way users can use also run number and some other parameters.
And they can store several TTrees in a file.

  <Tree>
      <TreeName>tree</TreeName>
      <TreeOutputFileName>"%s#.root",gAnalyzer->GetGSP()->GetFilePrefix()<TreeOutputFileName>
      <Branch>
        <BranchName>kine</BranchName>
        <RelatedFolder>Kinematics</RelatedFolder>
      </Branch>
  </Tree>
 

This change is quite big, and it affects all ROME users.
We have to discuss enough on it before changing.

(One choice is to remain run number base I/O.)

About configuration XML files.

Now we merged ROME and ARGUS.
So I expect that users often have several configuration files for one project.

But I realized many people don't know that they can specify configuration file.

So I propose following scheme.

* When a program starts, it searches all XML files located in current directory which has <Configuration> tag at the top.
* If it does not find any XML file, it asks user to generate new one.
   * ask configuration type [R/A/M/N]
   * ask filename. Default is romeConfig.xml
* If it find only one XML file, it reads the XML file.
* If it find several XML files, it asks user which file should be used with a prompt like,
     Which configuration file do you use ?
      [1] filename1.xml
      [2] romeConfig.xml
      [3] filename2.xml
      [q] quit program
* When user starts program with option -c, it asks creates new configuration anyway.
* When user starts program with option -i, it uses specified file.

done.
Program searches configuration XML file in ./ and ./config