Made some methods const

[lobis]

Changes required for rest-for-physics/geant4lib#41.

Some methods are made const, usage remains the same.

I also noticed abug in e6da8d7, perhaps the same bug is present elsewhere.

[lobis]

pipeline seems to fail giving an odd error with observables on the tree... I will look into it, no idea what can be, anyone has seen this before @rest-for-physics/core_dev ? Other libraries related to this PR pass the pipeline without issues.

[jgalan]

I have seen that before, because if something affects the observables previously registered at some point in time with the ones obtained now, then the pipeline will not succeed. However, there is no clue I could give since the reasons could be completely unconnected.

[juanangp]

This can also change the pipeline behaviour but it is indeed a bug, since energy should be double

[lobis]

yes, this is the only thing I can think of... but I don't think this method is used right? hopefully its not used anywhere since it was bugged.

[jgalan]

jgalan@sultan:~/rest-framework/source$ grep -rnw . -e "GetMostEnergeticHitInRange"
./libraries/track/src/TRestTrackBlobAnalysisProcess.cxx:215:        Int_t hit1 = hits->GetMostEnergeticHitInRange(0, nCheck);
./libraries/track/src/TRestTrackBlobAnalysisProcess.cxx:216:        Int_t hit2 = hits->GetMostEnergeticHitInRange(nHits - nCheck, nHits);

[juanangp]

pipeline seems to fail giving an odd error with observables on the tree... I will look into it, no idea what can be, anyone has seen this before @rest-for-physics/core_dev ? Other libraries related to this PR pass the pipeline without issues.

I have seen this as well, in my case the issue was in the code that I introduced. Note that if you push to a single library it doesn't trigger the pipeline for the framework, so it is very difficult to track the failure.

[juanangp]

Looking at the pipeline there is something wrong with the hits (left failure; right suceed)

[lobis]

Looking at the pipeline there is something wrong with the hits (left failure; right suceed)

I added back the bug, lets see if it gets fixed.

[juanangp]

I think I found something odd in rest-for-physics/connectorslib#10 perhaps related with the pipeline failure...

[lobis]

I think I found something odd in rest-for-physics/connectorslib#10 perhaps related with the pipeline failure...

I made some fixes but still pipeline fails...

[juanangp]

I made some fixes but still pipeline fails...

There is something nasty because the number of processed events is zero, which means that the error is different from before. Either a bug has been introduced with the new changes or elsewhere.

[juanangp]

Just checkout this branch and I realize that the pipeline is extremelly slow. Check below what I get while comparing with an older branch:

Doesn't seems related to a particular process, but all processes are slow. Do you know something that has been recently introduced which can explain this time difference? I guess should be under framework...

On the other hand, it would be good to check the running time in the pipeline validation to catch these kind of issues.

[jgalan]

At the end what it was making the pipeline fail?

Coming back to the processing time. Perhaps we should add as a validation pipeline that the processing time of a reference processing chain does not exceed a certain value. But not sure if it can be assured that it will be comparable at each runner.

[juanangp]

Test should be done in the same local machine. It could happen that the load from the server at a given time would affect, i imagine.

I am running in sultan, so I think that the issue is isolated to the code and not to the server.

[juanangp]

At the end what it was making the pipeline fail?

Aparently, we introduced another bug in rest-for-physics/connectorslib#10, which have to be fixed.

Coming back to the processing time. Perhaps we should add as a validation pipeline that the processing time of a reference processing chain does not exceed a certain value. But not sure if it can be assured that it will be comparable at each runner.

Yes, I was thinking of checking the cpu time, that I think is what the image that I posted before is doing. Can this info be added as metadata or so?

[jgalan]

Yes, I was thinking of checking the cpu time, that I think is what the image that I posted before is doing. Can this info be added as metadata or so?

I imagine we could add a new metadata member to TRestEventProcess::fAverageProcessingTime so that every process registers the time required for its own execution.

[lobis]

At the end what it was making the pipeline fail?

I have no idea, I made some changes to the pipeline scripts to have better debuging and suddenly it worked.

However I made the MakeBasicTree macro print the number of entries at the end and I get:

Finished `MakeBasicTree.C` macro. Number of entries: 0
(int) 0

so I think this is not good.

[lobis]

Looks like 53d73ec finally fixed the pipeline problems, thanks @juanangp.

So regarding that, this and all related PR can be merged.

However the problem with the speed... I can't think how this PR has anything to do with that, specially since it happens on all kinds of analysis proecsses, not only those that would be affected by this PR, what do you think about merging @jgalan @juanangp ?

[lobis]

@juanangp and I have checked together if these changes produce any performance change and haven't been able to reproduce, we made a clean install using these changes lobis-add-const-to-getters on all corresponding branches. Our conclusion is that this PR does not introduce any performance overhead so it can be merged.