-
Notifications
You must be signed in to change notification settings - Fork 9
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
Improving the hit-time and cluster filter processors with proper input and output collections #19
base: master
Are you sure you want to change the base?
Conversation
…o keep reco-hits constituents.
…eading the cuts properly from the digi config
Improving the cluster shape filters application
Tagging @madbaron , @kkrizka ; this PR implements a couple of things, but mainly we have adapted the timing hit filter so that it works also in cases where 1 sim hit <-> 1 reco hit assumption is not true anymore. I'm not 100% sure on the changes on the CMakeLists.txt file; I wonder if we have some good example we should follow for the current setup of packages. |
I don't think we have an example set up yet, but perhaps I can invoke @tmadlener and ask him to take a look specifically at CMakeLists.txt? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will probably have to rebase this onto the master branch after #20 (or merge that in). From a quick look, there is a chance for merge conflicts in the process. Let me know if you need help with that.
I have otherwise effectively only looked at the changes for the CMakeLists.txt
# load default settings from ILCSOFT_CMAKE_MODULES | ||
INCLUDE( ilcsoft_default_settings ) | ||
|
||
SET( COMPILER_FLAGS "-Wno-effc++" ) | ||
MESSAGE( STATUS "FLAGS ${COMPILER_FLAGS}" ) | ||
FOREACH( FLAG ${COMPILER_FLAGS} ) | ||
CHECK_CXX_COMPILER_FLAG( "${FLAG}" CXX_FLAG_WORKS_${FLAG} ) | ||
IF( ${CXX_FLAG_WORKS_${FLAG}} ) | ||
MESSAGE ( STATUS "Adding ${FLAG} to CXX_FLAGS" ) | ||
### We append the flag, so they overwrite things from somewhere else | ||
SET ( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${FLAG} ") | ||
ELSE() | ||
MESSAGE ( STATUS "NOT Adding ${FLAG} to CXX_FLAGS" ) | ||
ENDIF() | ||
ENDFOREACH() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep these. With #20 the warnings should be fixed so that output should be manageable again.
# needed for adding header files to xcode project | ||
IF(CMAKE_GENERATOR MATCHES "Xcode") | ||
FILE( GLOB_RECURSE library_headers "*.h" ) | ||
ADD_SHARED_LIBRARY( ${PROJECT_NAME} ${library_sources} ${library_headers}) | ||
ELSE() | ||
ADD_SHARED_LIBRARY( ${PROJECT_NAME} ${library_sources} ) | ||
ENDIF() | ||
|
||
INSTALL_SHARED_LIBRARY( ${PROJECT_NAME} DESTINATION lib ) | ||
|
||
# display some variables and write them to cache | ||
DISPLAY_STD_VARIABLES() | ||
ADD_LIBRARY( ${PROJECT_NAME} SHARED ${library_sources} ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No strong opinion on removing this. It's still present in iLCSoft/MarlinTrkProcessors. However what definitely has to stay is the installation of the library that we build, as otherwise we will not be able to use the processors when we use this into a software stack.
No description provided.