Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nan in _DRICH(Gas|Aerogel)IrtCherenkovParticleID_hypotheses.weight #1719

Open
wdconinc opened this issue Jan 22, 2025 · 10 comments
Open

Nan in _DRICH(Gas|Aerogel)IrtCherenkovParticleID_hypotheses.weight #1719

wdconinc opened this issue Jan 22, 2025 · 10 comments
Assignees

Comments

@wdconinc
Copy link
Contributor

Environment: (where does this bug occur, have you tried other environments)

  • Which branch (often main for latest released): main
  • Which version (or HEAD for the most recent on git): HEAD
  • Any specific OS or system where the issue occurs? CI
  • Any special versions of ROOT or Geant4? eic-shell

Steps to reproduce: (give a step by step account of how to trigger the bug)

With https://github.com/eic/EICrecon/actions/runs/12897784775/artifacts/2464891599 produced by https://github.com/eic/EICrecon/actions/runs/12897784775/job/35963715944

root [5] events->Scan("_DRICHGasIrtCherenkovParticleID_hypotheses.weight", "_DRICHGasIrtCherenkovParticleID_hypotheses.weight!=_DRICHGasIrtCherenkovParticleID_hypotheses.weight")
***********************************
*    Row   * Instance * _DRICHGas *
***********************************
*       22 *       11 *      -nan *
*       50 *        7 *      -nan *
*       65 *       31 *      -nan *
*       72 *        7 *      -nan *
*       76 *        3 *      -nan *
*       84 *       15 *      -nan *
*       85 *        7 *      -nan *
*       98 *        7 *      -nan *
***********************************
==> 8 selected entries

Expected Result: (what do you expect when you execute the steps above)

No nan.

Actual Result: (what do you get when you execute the steps above)

Nan.

@chchatte92
Copy link
Member

Hi @wdconinc I will look into it. But just to mention we will eventually get rid of this weight in future and include some chi^2 based PID in the coming months.

@wdconinc
Copy link
Contributor Author

Yes, but it would be nice if this could get fixed sooner and avoided in general at the irt stage by having some kind of more robust testing.

@chchatte92
Copy link
Member

I agree, however I checked several of my local reconstruction files. I have not seen there any nan values. I am checking with some DIS events. In principle, these weights are positive definite quantity. At least from mathematical pov.
In case I don't find anything strange at the IRT level I will probably use brute force only to store values if it is not a nan; at the EICRecon level. For example here:

out_hypothesis.weight = static_cast<decltype(edm4eic::CherenkovParticleIDHypothesis::weight)>(hyp_weight);

@wdconinc
Copy link
Contributor Author

only to store values if it is not a nan

No, that is ignoring the problem and hoping it goes away.

@chchatte92
Copy link
Member

Hi @wdconinc I am working on this issue. However, there are a couple of things which I don't really understand.
I am trying to print the PDG of the hypotheses along side the Cherenkov angle (polar angle in radian) and Z component of the momentum of the MCParticle.
What I see that, they all are electrons and funnily they have a negative z component of the momentum. This is really strange to me.
Any idea?
Image

@chchatte92
Copy link
Member

@deepaksamuel maybe you can also help me in figuring this issue out.

@wdconinc
Copy link
Contributor Author

Some of these electrons can be secondaries coming from somewhere in the forward region, e.g. backsplash from forward electromagnetic calorimeter.

@chchatte92
Copy link
Member

chchatte92 commented Jan 27, 2025

Some of these electrons can be secondaries coming from somewhere in the forward region, e.g. backsplash from forward electromagnetic calorimeter.

I thought about it; but, I expect no detected hits from them. As my mirror will reflect Cherenkov photons only from those particles which has a positive z component of the momentum. Otherwise I don't see any reason that the photons reach the sensor. Evidence of scintillation maybe !?!

@wdconinc
Copy link
Contributor Author

In any case, even if there are some electrons that you don't expect, the code should be robust and not return nan. That indicates you are (or IRT is) probably taking sqrt on negative numbers or dividing by an unchecked zero. I don't imagine it's an overflow, but you could look into that as well; that was an issue in the original implementation of the seed finder for straight tracks.

@wdconinc wdconinc reopened this Jan 27, 2025
@chchatte92
Copy link
Member

In any case, even if there are some electrons that you don't expect, the code should be robust and not return nan. That indicates you are (or IRT is) probably taking sqrt on negative numbers or dividing by an unchecked zero. I don't imagine it's an overflow, but you could look into that as well; that was an issue in the original implementation of the seed finder for straight tracks.

Great point. Will check this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants