Use GBL v3 within hps-java #958
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
still not working but all basic testing is passing, very confused
it compiles and runs but there is a logic bug preventing actual testing since I can't find the new test in the printouts
not very much of a memory leak so it wasn't crashing the program but cleaning it up will help avoid future memory-related crashes
The GblInterface is the java-JNA representation of libGBL.so which the other Gbl classes access in order to call the extern "C" functions from within that native library
|
Nice work, Tom. There is a lot of clean-up, code is more easy to read and I'm happy to see that some parts are more concise. Also, great catches on the memory leaks.
Maybe you have done this checks already, and from what I see from the code I do not see strong indication about expected changes, however this is a delicate part of the code, and best to make sure we get the same inputs to the minimizer. |
|
Good ideas on validation PF - I did do a quick comparison of the MPII files and found only floating-point differences between the my developments and what is currently on hps-java:master. I plan to rerun these validations in a more documented/formal way and post those results here so that everyone can be assured. I'll mark this PR as "Ready for Review" when I feel like I have no more To Dos and I think the branch is fully validated. |
Validation UpdateI've written up a quick CLI for the GBL JNA example so it is easier to compare them. Since this did not exist on Current GBL JNA ExampleThis is run on Full OutputDevelopments GBL JNA ExampleRunning the branch attached to this PR. Note: without the GBL v3 library provided, you will see an error about an undefined function. But in any case, I can provide the updated GBL v3 library. Full OutputDifference (or lack thereof)There is no difference between the two methods except the difference in timing. 917c916
< Time elapsed 104.728479 ms
---
> Time elapsed 108.581513 msTo make sure this timing difference is not systematic, I ran without debug printouts and 100k tries. |
Second Validation UpdateI am comparing this branch with The run command looks like the following. The only change made between the two runs was the branch of hps-java compiled into the jar, the path to the GBL version, and the name of the output file. The lcsim file is copied below if you wish to inspect it. These produced identical millepede binary files (using tracking_kf_alignment.lcsim<?xml version="1.0" encoding="UTF-8"?>
<lcsim xmlns:xs="http://www.w3.org/2001/XMLSchema-instance" xs:noNamespaceSchemaLocation="http://www.lcsim.org/schemas/lcsim/1.0/lcsim.xsd">
<!--
Steering file for running KF tracking and mille alignment
created: Nov 2022
@author Tom Eichlersmith <eichl008@umn.edu>
@author PF <pbutti@slac.stanford.edu>
-->
<execute>
<!-- Enable the following if re-processing lcio files -->
<driver name="PreCleanupDriver"/>
<driver name="RfFitter"/>
<driver name="EcalRunningPedestal"/>
<driver name="EcalRawConverter" />
<driver name="EcalTimeCorrection"/>
<driver name="ReconClusterer" />
<driver name="CopyCluster" />
<!--
SVT reconstruction drivers
SensorSetup associates hits with sensors without having to re-decode them.
TrackerHitDriver assocites clusters with tracker modules without having to re-cluster.
-->
<driver name="SensorSetup"/>
<driver name="TrackerHitDriver"/>
<!--
Kalman tracking
-->
<driver name="KalmanPatRecDriver"/>
<driver name="KalmanKinkFitDriver"/>
<driver name="KalmanToGBLDriver"/>
<!--
Use milli+GBL to prepare alignment data file for pede
-->
<driver name="SimpleGBLTrajAliDriverKF"/>
<!-- it has to be before GBLOutputDriver -->
<driver name="MultEvtVtx" />
<!--
Write out histograms
<driver name="GBLOutputDriverKF"/>
-->
<driver name="GBLOutputDriver" />
<driver name="CleanupDriver"/>
</execute>
<drivers>
<driver name="PreCleanupDriver" type="org.hps.analysis.dataquality.ReadoutCleanupDriver">
<!--Clean collections-->
<collectionNames>EcalCalHits EcalClusters EcalClustersCorr FinalStateParticles UnconstrainedV0Candidates UnconstrainedV0Vertices TargetConstrainedV0Candidates TargetConstrainedV0Vertices BeamspotConstrainedV0Candidates BeamspotConstrainedV0Vertices GBLKinkData GBLKinkDataRelations MatchedToGBLTrackRelations HelicalTrackHits HelicalTrackHitRelations MatchedTracks GBLTracks MatchedToGBLTrackRelations RotatedHelicalTrackHits RotatedHelicalTrackHitRelations TrackData TrackDataRelations TrackResiduals TrackResidualsRelations RotatedHelicalTrackHits RotatedHelicalTrackHitRelations StripClusterer_SiTrackerHitStrip1D </collectionNames>
</driver>
<driver name="RfFitter" type="org.hps.evio.RfFitterDriver"/>
<!-- Ecal reconstruction drivers -->
<driver name="EcalRunningPedestal" type="org.hps.recon.ecal.EcalRunningPedestalDriver">
<logLevel>CONFIG</logLevel>
</driver>
<driver name="EcalRawConverter" type="org.hps.recon.ecal.EcalRawConverter2Driver">
<!-- ecalCollectionName>EcalCalHits</ecalCollectionName -->
<!-- fixShapeParameter>true</fixShapeParameter -->
<!-- globalFixedPulseWidth>2.4</globalFixedPulseWidth -->
</driver>
<driver name="EcalTimeCorrection" type="org.hps.recon.ecal.EcalTimeCorrectionDriver"/>
<driver name="ReconClusterer" type="org.hps.recon.ecal.cluster.ReconClusterDriver">
<logLevel>WARNING</logLevel>
<outputClusterCollectionName>EcalClusters</outputClusterCollectionName>
</driver>
<driver name="CopyCluster" type="org.hps.recon.ecal.cluster.CopyClusterCollectionDriver">
<inputCollectionName>EcalClusters</inputCollectionName>
<outputCollectionName>EcalClustersCorr</outputCollectionName>
</driver>
<!-- SVT reconstruction drivers -->
<driver name="SensorSetup" type="org.hps.recon.tracking.SensorSetup" >
<readoutCollections>SVTRawTrackerHits</readoutCollections>
<fittedHitCollection>SVTFittedRawTrackerHits</fittedHitCollection>
</driver>
<driver name="TrackerHitDriver" type="org.hps.recon.tracking.DataTrackerHitDriver">
<neighborDeltaT>8.0</neighborDeltaT>
<saveMonsterEvents>false</saveMonsterEvents>
<thresholdMonsterEvents>200</thresholdMonsterEvents>
<debug>false</debug>
</driver>
<driver name="GBLOutputDriver" type="org.hps.recon.tracking.gbl.GBLOutputDriver">
<nHits>6</nHits> <!-- minimum hits -->
<outputPlotsFilename>${outputFile}_gblplots.root</outputPlotsFilename>
<bsZ>-7.5</bsZ>
<trackCollectionName>GBLTracks</trackCollectionName>
<!--the KF tracks refitted as GBL don't have kinks?? -->
<doGBLkinks>false</doGBLkinks>
<dataRelationCollection>""</dataRelationCollection>
<chi2Cut>9999</chi2Cut>
</driver>
<driver name="CleanupDriver" type="org.lcsim.recon.tracking.digitization.sisim.config.ReadoutCleanupDriver"/>
<driver name="MultEvtVtx" type="org.hps.recon.vertexing.MultipleEventsVertexingDriver">
<ntrks>100</ntrks>
</driver>
<driver name="KalmanPatRecDriver" type="org.hps.recon.tracking.kalman.KalmanPatRecDriver">
<!--<doDebugPlots>false</doDebugPlots>-->
<!-- <siHitsLimit>50</siHitsLimit> -->
<seedCompThr>0.05</seedCompThr>
<addResiduals>true</addResiduals>
<verbose>false</verbose>
</driver>
<!-- do front-back kink plots -->
<driver name="KalmanKinkFitDriver" type="org.hps.recon.tracking.kalman.KalmanKinkFitDriver">
</driver>
<!-- Form trajectories for MPII using the GBL algorithm, starting from Kalman Tracks -->
<driver name="KalmanToGBLDriver" type="org.hps.recon.tracking.gbl.KalmanToGBLDriver">
<!--<debug>true</debug>-->
</driver>
<!-- Form trajectories for MPII using the GBL algorithm -->
<driver name="SimpleGBLTrajAliDriverKF"
type="org.hps.recon.tracking.gbl.SimpleGBLTrajAliDriver" >
<!-- apply the track quality cuts -->
<enableAlignmentCuts>true</enableAlignmentCuts>
<!--
<doCOMAlignment>true</doCOMAlignment>
<minMom>0.5</minMom>
<maxMom>5.0</maxMom>
-->
<nHitsCut>6</nHitsCut> <!-- minimum number of hits to be included -->
<debugAlignmentDs>false</debugAlignmentDs>
<correctTrack>true</correctTrack> <!-- refit with GBL before doing Mille -->
<includeNoHitScatters>false</includeNoHitScatters>
<gblRefitIterations>0</gblRefitIterations>
<storeTrackStates>true</storeTrackStates>
<compositeAlign>true</compositeAlign>
<!-- <momC>4.55</momC> momentum constraint [GeV] -->
<constrainedFit>false</constrainedFit> <!-- apply momentum constraint -->
<constrainedBSFit>false</constrainedBSFit> <!-- apply beam spot constraint -->
<!-- <bsZ>-7.7</bsZ> beam spot z-coordinate, used to calculate beam spot -->
<trackSide>-1</trackSide> <!--hole-->
<writeMilleBinary>true</writeMilleBinary>
<milleBinaryFileName>${outputFile}_millepede.bin</milleBinaryFileName>
<writeMilleChi2Cut>5</writeMilleChi2Cut> <!-- max Chi2/Ndf to be included -->
<enableStandardCuts>false</enableStandardCuts>
<maxTrackChisq4hits>60.</maxTrackChisq4hits>
<maxTrackChisq5hits>60.</maxTrackChisq5hits>
<maxTrackChisq6hits>60.</maxTrackChisq6hits>
<inputCollectionName>KalmanFullTracks</inputCollectionName>
</driver>
<driver name="GBLOutputDriverKF" type="org.hps.recon.tracking.gbl.GBLOutputDriver">
<nHits>0</nHits> <!-- minimum hits -->
<outputPlotsFilename>${outputFile}_gblplots_from_kf.root</outputPlotsFilename>
<bsZ>-7.5</bsZ>
<trackCollectionName>KalmanFullTracks</trackCollectionName>
<trackResidualsRelColName>TrackResidualsKFtoGBLRelations</trackResidualsRelColName>
<!-- need to find relation collection for Kalma -->
<doGBLkinks>false</doGBLkinks>
<dataRelationCollection>""</dataRelationCollection>
<chi2Cut>9999</chi2Cut>
</driver>
</drivers>
</lcsim> |
pbutti
approved these changes
Apr 5, 2023
tracking/src/main/java/org/hps/recon/tracking/gbl/FittedGblTrajectory.java
Outdated
Show resolved
Hide resolved
tracking/src/main/java/org/hps/recon/tracking/gbl/FittedGblTrajectory.java
Outdated
Show resolved
Hide resolved
tracking/src/main/java/org/hps/recon/tracking/gbl/GblTrajectoryMaker.java
Show resolved
Hide resolved
tracking/src/main/java/org/hps/recon/tracking/gbl/HpsGblTrajectoryCreator.java
Outdated
Show resolved
Hide resolved
tracking/src/main/java/org/hps/recon/tracking/gbl/SimpleGBLTrajAliDriver.java
Outdated
Show resolved
Hide resolved
tracking/src/main/java/org/hps/recon/tracking/gbl/SimpleGBLTrajAliDriver.java
Outdated
Show resolved
Hide resolved
tracking/src/main/java/org/hps/recon/tracking/gbl/SimpleGBLTrajAliDriver.java
Outdated
Show resolved
Hide resolved
tracking/src/main/java/org/hps/recon/tracking/gbl/SimpleGBLTrajAliDriver.java
Show resolved
Hide resolved
tracking/src/main/java/org/hps/recon/tracking/gbl/GBLexampleJna1.java
Outdated
Show resolved
Hide resolved
|
Would it be possible to add one or more tests for these changes that can run with the hps-java build? |
|
I added running the example to the tests - do you mean running over actual data files? |
I meant an actual junit TestCase that could run with the build automatically. Is this tricky due to the dependency on the GBL system library? |
|
I know what you meant and I did implement more tests into the GblJna test case: |
|
It will be interesting to see how this test fares on systems without GBL accessible - hopefully it just reports it as a failure without anything more. |
resolved conflict form tabbing different in SimpleGBLTrajAliDriver, tabbing updated in PR #972 when PF moved where the closing bracket to compositeAlign was
|
I resolved the conflict created by different tabbing relative to #972 and reran the full tests (including the updated GBL tests). Looks all good! |
|
While cleaning up the GblPoints and helixes is not as much memory as the original GblTrajectory it still is substantial. Anecdotally, earlier this week I was running on 2016 data that took ~2hrs to run. I could run manually at SDF but my batch jobs were crashed, being evicted since they went over the memory limit. To test this, I manually monitored the memory of a single run by watching Running with hps-java master, I gave the JVM 3G and saw that, by the end of the run, |
|
Do you have CPU times and memory usage when running the native Java version? |
|
Again anecdotally... The native java (i.e. non-JNA) version stays within the scope of the memory allocated to the JVM (so |
cbravo135
approved these changes
Apr 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main motivation for this update was to align the function signatures between the updated GBL v3
extern "C"functions and the functions in the JNA interface classes. In the course of developing this, I learned more about JNA and was able to make a few other enhancements.GblInterfacewhich directly represents thelibGBL.solibrary it loads and calls theextern "C"functions defined within it.Going to wait on my DESY GitLab merge request so that the GBL-JNA files can be linked with a specific version of GBL from the central public repository. For now, if you want to test this, you will need to compile thejava-integrationbranch of my fork of GBL.My DESY GitLab merge request was accepted and a new release has been made. Compile the updated GBL v3 with the JNA wrappers included by using v3.1 or newer.
Running To Do
Move non-JNA GBL classes to alegacysub-package avoid future confusionmvn javadoc:javadocto make sure they are picked up)Validations To Do