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

Bump to v1.2 #1197

Merged
merged 3 commits into from
Sep 8, 2023
Merged

Bump to v1.2 #1197

merged 3 commits into from
Sep 8, 2023

Conversation

pbutti
Copy link
Contributor

@pbutti pbutti commented Sep 6, 2023

I am updating ldmx-sw, here are the details.

Bump tracking to stable release v1.2

Check List

  • I successfully compiled ldmx-sw with my developments

  • I ran my developments and the following shows that they are successful.

< put plots or some other proof that your developments work >

  • I attached any sub-module related changes to this PR.

Copy link
Contributor

@bryngemark bryngemark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me -- it's hard to really comment. I was wondering though, in the example config/joboptions, are you mostly using parameter settings that are already the default? If you want to make it a great example, indicating only those settings you'd typically want to play with or change would make it more useful to the beginner. Also, I have no idea if this is the case, but if there are typical sets of parameter changes that would go together, they could be bundled up in a predefined function (like the different simulation types are, for example) to minimize the risk that a user picks bad parameter combinations.

@pbutti
Copy link
Contributor Author

pbutti commented Sep 7, 2023

@bryngemark
Hi Lene Kristian,
I'm using parameters that are default, although many of those would need to be properly tuned for ensuring a reasonable reconstruction: I'm not there yet.
The parameters are separated by processor, so in principle one would be able to to change each processor separately to act on different stages of the reconstructions.
In principle one could start define some quality cuts marked by strings, i.e. loose/tight tracks that would bundle together a set of parameters.
I can not this down and try to include it in the new library version.

@tomeichlersmith
Copy link
Member

@pbutti Can you post a config (or configs if you did multiple steps) you used to run the tracking? I tried the following and got an error about using a pre-existing product. These errors usually crop up from running processors in the wrong order (or omitting ones that are necessary).

Process

Commands

# after building and install this branch
ldmx fire sim.py
ldmx fire trk.py justSim_10_events.root

Error

# after GDML parsing printouts
 [ fire ] 4 : [ProductExists] : A product named 'TaggerTruthTracks' already exists in the event (has been loaded by a previous producer in this process).
  at /export/scratch/users/eichl008/ldmx/ldmx-sw/Framework/include/Framework/Event.h:160 in add
Stack trace: 
    0 void framework::Event::add<std::vector<ldmx::TruthTrack, std::allocator<ldmx::TruthTrack> > >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<ldmx::TruthTrack, std::allocator<ldmx:
:TruthTrack> >&) + 2898 
    1 tracking::reco::TruthSeedProcessor::produce(framework::Event&) + 7777 
    2 framework::Process::process(int, framework::Event&) const + 113 
    3 framework::Process::run() + 693 
    4 main + 461 addr2line: 'fire': No such file

sim.py

This is just a copy of SimCore/test/basic.py

from LDMX.Framework import ldmxcfg
# create my process object
p = ldmxcfg.Process( "test" )
# how many events to process?
import sys
p.maxEvents = 10
if len(sys.argv) > 1 :
    p.maxEvents = int(sys.argv[1])
# we want to see every event
p.logFrequency = 1
p.termLogLevel = 0
# Set a run number
p.run = 9001
# we also only have an output file
p.outputFiles = [ "justSim_" + str(p.maxEvents) + "_events.root" ]
from LDMX.SimCore import simulator as sim
import LDMX.Ecal.EcalGeometry
import LDMX.Hcal.HcalGeometry
mySim = sim.simulator( "mySim" )
mySim.setDetector( 'ldmx-det-v14' , True )
# Get a pre-written generator
from LDMX.SimCore import generators as gen
mySim.generators.append( gen.single_4gev_e_upstream_tagger() )
# add your configured simulation to the sequence
mySim.description = 'Basic test Simulation'
p.sequence.append( mySim )

trk.py

import argparse
from pathlib import Path

parser = argparse.ArgumentParser()
parser.add_argument('sim_file', type=Path)
args = parser.parse_args()

from LDMX.Framework import ldmxcfg
from LDMX.Tracking import examples

p = examples.single_e_track_recon_10um()

from LDMX.Tracking import geo

p.inputFiles = [ str(args.sim_file) ]
p.outputFiles = [ args.sim_file.stem + '_tracked.root' ]

@tomeichlersmith
Copy link
Member

It looks like the TruthSeedProcessor should only be run once. I'm basing this on the fact that it adds the truth track collections for both the recoil and tagger.

https://github.com/LDMX-Software/Tracking/blob/18f443683a92d5015d0c128033d59ad42e827f36/src/Tracking/Reco/TruthSeedProcessor.cxx#L360-L362

But it only takes either the RecoilSimHits or the TaggerSimHits as input? We might need to update Tracking and make a new release of it before it can be merged.

@pbutti
Copy link
Contributor Author

pbutti commented Sep 7, 2023

The p = examples.single_e_track_recon_10um() is not up-to-date. The example that should be used to build a tracking job is reco.py inside Tracking/python. In there you will find all the single producers configuration.

The truth seeder runs only once. The hit inputs have been superseded by the usage of scoring plane information.

Copy link
Member

@tomeichlersmith tomeichlersmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alrightie, after conferring with @pbutti , I made some usability patches and then bumped the version patch number. Now I am happy with merging this in and start the work of integrating it into the CI.

@tomeichlersmith tomeichlersmith merged commit a64dccd into trunk Sep 8, 2023
1 check passed
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

Successfully merging this pull request may close these issues.

3 participants