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

buildITkGeometry in itk.py doesn't produce hits #1149

Closed
timadye opened this issue Feb 8, 2022 · 9 comments · Fixed by #1155
Closed

buildITkGeometry in itk.py doesn't produce hits #1149

timadye opened this issue Feb 8, 2022 · 9 comments · Fixed by #1155
Labels
Bug Something isn't working Impact - Minor Nuissance bug and/or affects only a single module
Milestone

Comments

@timadye
Copy link
Contributor

timadye commented Feb 8, 2022

I used buildITkGeometry to create a TGeoDetector geometry for the ATLAS ITk. This looks OK, except when used by Fatras, it doesn't generate any hits.

If instead, I specify the geometry definition with

TGeoDetector.create(jsonFile="acts-detector-examples/atlas/itk-hgtd/tgeo-atlas-itk-hgtd.json", ...)

hits are generated without problem.

I'll continue to try to find a difference in the returned detector and trackingGeometry objects from these two methods, but thought it worth flagging up now in case someone has a better idea.

@timadye timadye added the Bug Something isn't working label Feb 8, 2022
@timadye timadye added this to the next milestone Feb 8, 2022
@timadye timadye added the Impact - Minor Nuissance bug and/or affects only a single module label Feb 8, 2022
@paulgessinger
Copy link
Member

I assume aside from the geometry source, it's the exact same script?

@timadye
Copy link
Contributor Author

timadye commented Feb 8, 2022

Yes, identical script. I added the json geometry building to itk.py and switched between them with a flag. Then used particle_gun.py and fatras.py to make the tests.

@timadye
Copy link
Contributor Author

timadye commented Feb 8, 2022

Following @paulgessinger's suggestion, I ran a single particle (10 GeV muon, eta=0.5) through the two geometries with verbose logging:

created with buildITkGeometry: fatras_pybind11_config.log (no hits created)
created with tgeo-atlas-itk-hgtd.json: fatras_json_config.log (17 hits created)

The most immediately obvious difference is that the first detector Fatras encounters: HGTD::PositiveEndcap (Python) vs BeamPipe::Barrel (JSON). The former doesn't see any other detectors, while the latter sees InnerPixels::Barrel, OuterPixels::Barrel, Strips::Barrel as expected. The HGTD shouldn't be anywhere in this particle's path!

This was run with the current main branch, with @noemina's beampipe fixes to both Python and JSON.

@timadye
Copy link
Contributor Author

timadye commented Feb 8, 2022

I guess the HGTD is being treated as a barrel detector, so the rRange 0-1050mm kicks in straight away. But it has:

layers=LayerTriplet(positive=True, central=False, negative=True)

@timadye
Copy link
Contributor Author

timadye commented Feb 8, 2022

I found a quick workaround. If I comment out the HGTD Volume() from the Python config, I get identical results to the JSON definition.
Obviously that would skip the HGTD for the forward tracks that encounter it, but I'm not so concerned with the HGTD at the moment.

@timadye
Copy link
Contributor Author

timadye commented Feb 9, 2022

Hi @noemina,

To quickly test this out, you can use timadye:pybind6. This has some other changes on top of acts-project:main, but also includes the option to create the geometry from JSON so you can compare the two.

There's also a top-level script, full_chain_itk.py, that you can run just like this:

Examples/Scripts/Python/full_chain_itk.py

Currently it just does particle_gun, fatras, and digitization. To test just this effect, you can comment out addDigitization() call.

That produces hits.root with 115 hits, but all have volume_id=25 (HGTD). (This is a bit different to what I reported before: with these selections, you do get hits, just not in the ITk.)

You can compare with the JSON configuration with this branch, by specifying jsonconfig=True. Other possible debugging options are:

detector, trackingGeometry, decorators = itk.buildITkGeometry(
    geo_dir, jsonconfig=True, material=False, logLevel=acts.logging.VERBOSE
)

I also tested with changes to the addParticleGun options to fully specify a single particle with:

s = addParticleGun(
    s,
    MomentumConfig(10.0 * u.GeV, 10.0 * u.GeV, True),
    EtaConfig(0.5, 0.5, False),
    ParticleConfig(1, acts.PdgParticle.eMuon, False),
    rnd=rnd,
)

You can add acts.examples.Sequencer(..., logLevel=acts.logging.VERBOSE) to get the logfiles I posted above.

I hope that helps,
Tim.

@noemina
Copy link
Contributor

noemina commented Feb 10, 2022

Hello @timadye,
I found the problem :)
You misspelled positive for the zRange definition of the HGTD. As a consequence, the HGTD positive side got extended from -std::numeric_limits<double>::max() to std::numeric_limits<double>::max(), hence invading the Pixel and Strip volumes. If you fix the line posted below, everything will work as expected :)

zRange=LayerTriplet(
        negative=(-4000 * u.mm, -3000 * u.mm),
        postive=(3000 * u.mm, 4000 * u.mm),
        ),

Cheers,
Noemi

@paulgessinger
Copy link
Member

I'm guessing that was on me, actually. My bad.

@timadye
Copy link
Contributor Author

timadye commented Feb 10, 2022

Thanks for the fix @noemina! That's excellent.

I'm surprised this didn't flag an error. Is it possible to give an unexpected keyword argument error something?

Thanks,
Tim.

@kodiakhq kodiakhq bot closed this as completed in #1155 Feb 10, 2022
kodiakhq bot pushed a commit that referenced this issue Feb 10, 2022
Improving the description of the ITk geometry parameters in the python file.
This fixes as well the issue #1149, caused by the wrong configuration of the HGTD extends.

Fixes #1149
@paulgessinger paulgessinger modified the milestones: next, v17.1.0 Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Impact - Minor Nuissance bug and/or affects only a single module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants