Re-ordered source file and library structure

[nkx]

I have updated the source file structure according to the concept of v2.3.0 update. Sign in · GitLab

It passes the pipeline verification. Sign in · GitLab

The new library order is shown as following. There are 9 libraries and 1 packages in the dependence tree.

Several problems:

1. Muon library is dependent on raw library. This is not plain. I will work on it, and make it to be dependent on DetectorLib
2. I added RestGarfield library, which contains a very old process TRestGarfieldDriftProcess and TRestGeometry. It is very ugly to have two libraries dependent on Garfield. I still like to release RestDetector library’s dependence, by moving TRestGas into RestGarfield library. Otherwise we would merge RestGarfield library into RestDetector library, leaving an external dependence(which could be unstable) for this infrastructure class.
3. RestDetectorLib should be compiled by default, so actually we can regard it to be at same level as RestFramework. Then the libraries are all plain, meaning that they have no dependence with each other, which is very good for me.
4. restDecay0 and restPsql is working differently as other blue colored libraries. They implements a base class with the functionality of a package. They do not add any process or events for analysis. But anyway, I think we can call them all as “library”

I imagine the final library structure to be:

[jgalan]

I think we should not organise our selves by how libraries link to external libraries. That said, I believe RestGarfield and RestDecay0 seem to be plugins, if we understand by a plugin a compilation option of a library. TRestGarfieldDriftProcess is probably not maintained and used, and not operative I would say. TRestGeometry is not used anymore. But those processes using Garfield can be appropriately encapsulated in REST_GARFIELD #define. We have other libraries using Garfield now, as RestAxionLib, so I don’t see the sense to have a RestGarfieldLib. Similar reasoning for restDecay0 I believe, we could just have a compilation option to enable/disable.

Not sure about restPsql but @lobis is working on a a package, restSQL that populates a SQL database. I don’t know what it is in restPsql but if it is a library I would expect that they would be functions and interfaces to access/create and manage an SQL database, and therefore probably would be a plugin in the framework so that any library or package would benefit from accessing an SQL database?

Then, I understand your concern about RestFramework and detector library being together. However, they should still sit in different directories with different CMakeLists and we should be able to go inside the /framework/ directory, build and install REST, and go to the /detector/ directory, and build and install the detector library.

Then, we can have a main CMakeLists that makes everything, build and install the framework, build and install the detector library, and build and install any library found under /libraries/ directory.

We need still to make something to require a minimum REST version. As discussed in Sign in · GitLab

So, we should probably be able to do in the CMakeLists of a particular library:

rest_minimum_required( 2,2,12)
rest_libs_required( libRestDetector)

Those methods, I guess, should be written in a FindREST.cmake file.

[jgalan]

Also, from our last phone meeting, I wanted to make it clear what a REST library is: It is a set of TRestMetadata, and TRestEventProcess objects that work around 1 or 2 TRestEvent data types. I believe that will make us keep working in a coherent way.

We should stick to that definition or concept just for clarity, and potential scalability. If something does not fit with the REST library definition, I am sure it will fit within the core framework.

But if a REST process needs any external/additional library to work, that is an option that must be included in the library. We cannot create a library to put in there anything that contains that dependence.

[nkx]

@jgalan

if we understand by a plugin a compilation option of a library.

For restDecay0 it is no problem, if we want to switch it on/off, we just recompile restG4. However, for restPsql and restGarfield, If we still apply this concept, it changes our fundamental libraries.

@jgalan

I don’t know what it is in restPsql but if it is a library I would expect that they would be functions and interfaces to access/create and manage an SQL database, and therefore probably would be a plugin in the framework so that any library or package would benefit from accessing an SQL database?

Really it is not good to have compilation options in RestFramework. That’s why I propose to extract those compilation options into individual libraries. By following some naming convention, the extracted “plugin libraries” can work similarly as if we turned on the compilation options, while doesn’t change anything to the original library.

The points of making restPsql a framework compilation option:

1. The additional compilation options makes it complex. We already have options to choose install libraries for different analysis task, why we still add compilation options to them? Having only one option is more straight forward.
2. I currently don’t have time to maintain it, it lacks verification. In future when we meet the demand of accessing the database with TRestRun, we have to update the code. But we don’t want RestFramework to be frequently updating, right?
3. Different experiments(or collaborations) have different sql table structure when they setup their database. They shall feel free to change the code themselves. The changed code shall of course not be in the framework.
4. What if people want to access mySQL, SQLServer? I don’t want to verbosely add multiple compilation options(c++ preprocessors: #ifdef, #else, #endif) in TRestDataBase.cxx. The code will be very ugly.
5. Switching on/off the compilation flag leads to a recompilation for the dependent libraries. Unless we write the code well(making all the options effect only the cxx file)

These points are same for other future plugins. I believe PostGreSQL’s interface won’t change much. But for others I have to add another point:

6. The interface of external packages could change with time. If it happens, and we integrated the package with compilation option, then we need to change the whole library and increase its version in order to catch up with the new interface. This is leads to another unnecessary update of infrastructural libraries. All the dependent libraries needs to be recompiled. On the other hand, if we extract those compilation options into individual libraries, we can simply focus on the core functionality. Only those “plugin libraries” needs to be maintained.

In one word, we shouldn’t integrate the external packages with compilation option inside the infrastructural libraries(i.e. RestFramework and RestDetector). For other libraries, It is OK to use plugin.

@jgalan

Then, I understand your concern about RestFramework and detector library being together. However, they should still sit in different directories with different CMakeLists and we should be able to go inside the /framework/ directory, build and install REST, and go to the /detector/ directory, and build and install the detector library.

That’s what I did.

@jgalan

We need still to make something to require a minimum REST version. As discussed in Sign in · GitLab

OK. I can do this.

@jgalan

But if a REST process needs any external/additional library to work, that is an option that must be included in the library. We cannot create a library to put in there anything that contains that dependence.

Yes, but not for RestFramework and RestDetector library

[nkx]

@jgalan

TRestGarfieldDriftProcess is probably not maintained and used, and not operative I would say.

@jgalan

We have other libraries using Garfield now, as RestAxionLib, so I don’t see the sense to have a RestGarfieldLib.

I heard someone in my group is simulating electron drift process with Garfield. It can give you the detailed trace of the drift of one single electron under specified electric field. So I guess there are much to dig from Garfield, and we may keep the RestGarfieldLib as a more advanced application of it. Maybe we change the library name?

It is also OK to have multiple libraries using Garfield.

[jgalan]

@nkx

So I guess there are much to dig from Garfield, and we may keep the RestGarfieldLib as a more advanced application of it.

Does the new TRestGarfieldDriftProcess operate over a new event data type? If it operates in a TRestHitsEvent or future TRestDetectorHitsEvent, then it would be more appropriate that this new process or bunch of processes that operate on TRestHitsEvent would be in the detector lib. Because doing that it puts together all the processes that can be operated without changing the event type.

It means we will be able to inter-connect any process, existing or new one, it will be clear that they can be connected because they will belong to the same library. Then, it will be clear to which other libraries it may export its event type, as for example transforming Hits to RawHits.

Thats why I believe we should group libraries based on the event type they operate on. The magic is in the possibility to inter-connect processes created by different people. If those Garfield processes end-up in the detector library, there are higher chances that I will have a look at them, because I will think it is connected, and not an isolated library for a very particular purpose.

---

Of course, if what you have in mind for RestGarfieldLib, is something that doesn’t connect at all with RestDetectorLib even if it uses TRestHitsEvent, OR if you need to define a very different data type so that we cannot adapt the existing TRestHitsEvent, then a new library would be justified. But, of course, RestGarfieldLib should be changed by a more meaning full name.

Still, I imagine is not the case, because if you are propagating electrons in a drift volume, at some point you will be willing to transform the hits to signal using the readout.

In the other hand I could imagine that you do a completely parallel development, using completely new tools to reproduce the readout, etc, etc. But I would not like it evolving that way. At the end would be like working in the same boat but paddling in different senses.

[jgalan]

@nkx

restGarfield , If we still apply this concept, it changes our fundamental libraries.

This I do not understand, how it changes our fundamental libraries. Actually it is operating as plugin (or external library). When we do cmake, we need to add DREST_GARFIELD=ON to enable it.

@nkx

Really it is not good to have compilation options

Why this is not good? It is like that, ROOT has hundreds on compilation options.

Building ROOT from source

Why would be not good?

Of course, you need to encapsulate the code. And, by default, libraries CMake compilation should take the options used in building RESTFramework. So that if SQL interfaces using a external library are used, then the library links. But not sure if there is a real need to link if the interface at RESTFramework is protected in each method and drops a warning message in case RESTFramework library was not linked with the external library.

And I would say also, that if CMake detects that Garfield is properly installed in the system it enables REST_GARFIELD automatically.

[jgalan]

@nkx

The points of making restPsql a framework compilation option:

restPsql would not be a framework compilation option, the option would be PostgreSQL. I imagine that we can also get the compilation option from ROOT and automatically compile with PostgreSQL if ROOT is compiled against PostgreSQL.

In ROOT this is the option

pgsql	ON	PostgreSQL support, requires libpq

@nkx

What if people want to access mySQL, SQLServer? I don’t want to verbosely add multiple compilation options(c++ preprocessors: #ifdef, #else, #endif) in TRestDataBase.cxx. The code will be very ugly.

Perhaps we may have a TRestPSQLDataBase and another TRestMySQLDataBase, that maybe inherit from a common interface?

Then, to avoid using #ifdef … #endif we could perhaps add a flag in CMakeLists so that TRestMySQLDataBase is only compiled if mysql libraries are enabled.