APRIL and SDHCALContent integration #29

tanguypasquier · 2024-07-10T15:11:33Z

BEGINRELEASENOTES

Add ability to link with new PFA contents : SDHCALContent for SDHCAL specific corrections and APRILContent for APRIL PFA algorithms.
Allow to compile with or without linking to new PFA Contents, default is to build without new contents.
Add processor option to switch between Pandora and APRIL PFAs.
ENDRELEASENOTES

tmadlener

Sorry for the long delay, this slipped through during summer / vacation time. Overall this looks pretty good to me. I think we should also try and get APRILContent and SDHCalContent into the Key4hep nightly builds to iron out all potential dependency issues and to also enable at least building these options in CI.

I have effectively mainly a few minor comments for the cmake configuration and some code formatting. The biggest thing I would change is the implementation of the usage of the hit factory that currently initializes creates two factories, when only one would be necessary.

tmadlener · 2024-08-28T12:30:43Z

CMakeLists.txt

+    IF( APRILContent_FOUND )
+        INCLUDE_DIRECTORIES( SYSTEM ${APRILContent_INCLUDE_DIRS} )
+        LINK_LIBRARIES( ${APRILContent_LIBRARIES} )
+        ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )
+        ADD_DEFINITIONS( "-DAPRILCONTENT" )
+    ENDIF()
+    IF( mlpack_FOUND )
+        INCLUDE_DIRECTORIES( SYSTEM ${mlpack_INCLUDE_DIRS} )
+        LINK_LIBRARIES( ${mlpack_LIBRARIES} )
+        ADD_DEFINITIONS ( ${mlpack_DEFINITIONS} )
+    ENDIF()


Suggested change

IF( APRILContent_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${APRILContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${APRILContent_LIBRARIES} )

ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )

ADD_DEFINITIONS( "-DAPRILCONTENT" )

ENDIF()

IF( mlpack_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${mlpack_INCLUDE_DIRS} )

LINK_LIBRARIES( ${mlpack_LIBRARIES} )

ADD_DEFINITIONS ( ${mlpack_DEFINITIONS} )

ENDIF()

INCLUDE_DIRECTORIES( SYSTEM ${APRILContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${APRILContent_LIBRARIES} )

ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )

ADD_DEFINITIONS( "-DAPRILCONTENT" )

INCLUDE_DIRECTORIES( SYSTEM ${mlpack_INCLUDE_DIRS} )

LINK_LIBRARIES( ${mlpack_LIBRARIES} )

ADD_DEFINITIONS ( ${mlpack_DEFINITIONS} )

If you have REQUIRED in your find_package call there is no need to check again if it is actually found, because CMake will actually stop if it is not found.

Suggested change

IF( APRILContent_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${APRILContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${APRILContent_LIBRARIES} )

ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )

ADD_DEFINITIONS( "-DAPRILCONTENT" )

ENDIF()

IF( mlpack_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${mlpack_INCLUDE_DIRS} )

LINK_LIBRARIES( ${mlpack_LIBRARIES} )

ADD_DEFINITIONS ( ${mlpack_DEFINITIONS} )

ENDIF()

IF( APRILContent_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${APRILContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${APRILContent_LIBRARIES} )

ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )

ADD_DEFINITIONS( "APRILCONTENT" )

ENDIF()

IF( mlpack_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${mlpack_INCLUDE_DIRS} )

LINK_LIBRARIES( ${mlpack_LIBRARIES} )

ADD_DEFINITIONS ( ${mlpack_DEFINITIONS} )

ENDIF()

Deleted the IF(... FOUND) in the commit "CMakeLists updates" (d49c7a0)

I tried to delete the -D in the add definitions but it seems that the compiler wasn't understanding the definition as a macro anymore and was trying to find it as file or directory, thus creating errors during compilation

The special handling of adding -D might only be available for target_compile_definitions(?) @andresailer

tmadlener · 2024-08-28T12:32:04Z

CMakeLists.txt

+    IF( SDHCALContent_FOUND )
+        INCLUDE_DIRECTORIES( SYSTEM ${SDHCALContent_INCLUDE_DIRS} )
+        LINK_LIBRARIES( ${SDHCALContent_LIBRARIES} )
+        ADD_DEFINITIONS ( ${SDHCALContent_DEFINITIONS} )
+        ADD_DEFINITIONS( "-DSDHCALCONTENT" )
+    ENDIF()


Suggested change

IF( SDHCALContent_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${SDHCALContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${SDHCALContent_LIBRARIES} )

ADD_DEFINITIONS ( ${SDHCALContent_DEFINITIONS} )

ADD_DEFINITIONS( "-DSDHCALCONTENT" )

ENDIF()

INCLUDE_DIRECTORIES( SYSTEM ${SDHCALContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${SDHCALContent_LIBRARIES} )

ADD_DEFINITIONS ( ${SDHCALContent_DEFINITIONS} )

ADD_DEFINITIONS( "-DSDHCALCONTENT" )

If you have REQUIRED in your find_package call there is no need to check again if it is actually found, because CMake will actually stop if it is not found.

Suggested change

IF( SDHCALContent_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${SDHCALContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${SDHCALContent_LIBRARIES} )

ADD_DEFINITIONS ( ${SDHCALContent_DEFINITIONS} )

ADD_DEFINITIONS( "-DSDHCALCONTENT" )

ENDIF()

IF( SDHCALContent_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${SDHCALContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${SDHCALContent_LIBRARIES} )

ADD_DEFINITIONS ( ${SDHCALContent_DEFINITIONS} )

ADD_DEFINITIONS( "SDHCALCONTENT" )

ENDIF()

cmake will add -D when needed, and in places where we have to treat definitions programatically it is easier if we don't have to deal with some with -D and some without.

Same as above

tmadlener · 2024-08-28T12:36:33Z

cmake/Findmlpack.cmake

+FIND_PACKAGE_HANDLE_STANDARD_ARGS(mlpack DEFAULT_MSG mlpack_CORE_HPP_DIR)
+
+IF(NOT mlpack_FOUND)
+  MESSAGE(FATAL_ERROR "The mlpack package not found.")
+ENDIF()


Suggested change

FIND_PACKAGE_HANDLE_STANDARD_ARGS(mlpack DEFAULT_MSG mlpack_CORE_HPP_DIR)

IF(NOT mlpack_FOUND)

MESSAGE(FATAL_ERROR "The mlpack package not found.")

ENDIF()

include(FindPackageHandleStandardArgs)

FIND_PACKAGE_HANDLE_STANDARD_ARGS(mlpack DEFAULT_MSG mlpack_CORE_HPP_DIR)

Make sure that FIND_PACKAGE_HANDLE_STANDARD_ARGS is actually always available

FIND_PACKAGE_HANDLE_STANDARD_ARGS already does the correct handling of REQUIRED, QUIET, ... (the arguments that are passed to find_package).

Fixed in the "Fixed Findmlpack.cmake" (d8f1d2c)

cmake/Findmlpack.cmake

tmadlener · 2024-08-28T12:52:16Z

include/DDPandoraPFANewProcessor.h

+/**
+*  @brief  Register the LCContent plugins outside their original function. Temporary fix. Would be better to change the plugins registration directly in LCContent
+*/
+pandora::StatusCode PandoraHack(const pandora::Pandora &pandora);


Suggested change

pandora::StatusCode PandoraHack(const pandora::Pandora &pandora);

pandora::StatusCode AprilLCContentPluginRegistration(const pandora::Pandora &pandora);

(or a more fitting name)

Would at least hide a bit that this is a "hack", while also making it rather clear from the function name what is happening (on a high level).

I updated the name in the commit "Fixed the factory issue" (d793e70)

tmadlener · 2024-08-28T12:53:56Z

src/DDCaloHitCreator.cc

@@ -70,6 +80,7 @@ DDCaloHitCreator::~DDCaloHitCreator()

 pandora::StatusCode DDCaloHitCreator::CreateCaloHits(const EVENT::LCEvent *const pLCEvent)
 {
+    ChooseFactory(); //New method to choose the factory to use


Does this have to happen every event? This could be done only once in init where all the necessary information should also be available, right?

Indeed, I moved it to the DDCaloHitCreator constructor so it's now called only once in the init

tmadlener · 2024-08-28T12:56:51Z

include/DDCaloHitCreator.h

+#endif
+
+    //Added by T.Pasquier
+    pandora::ObjectFactory<object_creation::CaloHit::Parameters, object_creation::CaloHit::Object>* caloHitFactory; //General factory to initialize


Suggested change

pandora::ObjectFactory<object_creation::CaloHit::Parameters, object_creation::CaloHit::Object>* caloHitFactory; //General factory to initialize

pandora::ObjectFactory<object_creation::CaloHit::Parameters, object_creation::CaloHit::Object>* m_caloHitFactory; //General factory to initialize

To make it easier to spot that this is a member variable. (Requires some changes in the .cc file as well)

Given the overall usage pattern in the code, I would actually make this

std::unique_ptr<pandora::ObjectFactory<object_creation::CaloHit::Parameters, object_creation::CaloHit::Object>> m_caloHitFactory{nullptr};

(#include <memory> for unique_ptr)

Then ChooseFactory would become something like

#ifdef APRILCONTENT if(m_settings.m_useAPRIL) { caloHitFactory = std::make_unique<april_content::CaloHitFactory>(); } else #endif { caloHitFactory = std::make_unique<pandora::PandoraObjectFactory<object_creation::CaloHit::Parameters, object_creation::CaloHit::Object>>(); }

And the DefaultCaloFactory as well as the m_pAPRILCaloHitFactory member variables could go.

Usage in the implementation would then be

PANDORA_THROW_RESULT_IF(pandora::STATUS_CODE_SUCCESS, !=, PandoraApi::CaloHit::Create(m_pandora, caloHitParameters, *m_caloHitFactory));

Thank you very much! I fixed it and it works perfectly

src/DDPandoraPFANewProcessor.cc

andresailer · 2024-08-28T13:46:08Z

cmake/Findmlpack.cmake

mlpack should come with a cmake config file, so this findmlpack should not be needed
https://github.com/mlpack/mlpack/blob/master/CMake/mlpack-config.cmake.in

@tanguypasquier did you check, whether you can simply remove this file and things keep working?

I indeed tried to simply remove the file but it generates some errors during the compilation. I get :

`By not providing "Findmlpack.cmake" in CMAKE_MODULE_PATH this project has
asked CMake to find a package configuration file provided by "mlpack", but
CMake did not find one.

Could not find a package configuration file provided by "mlpack" with any
of the following names:

mlpackConfig.cmake
mlpack-config.cmake

Add the installation prefix of "mlpack" to CMAKE_PREFIX_PATH or set
"mlpack_DIR" to a directory containing one of the above files. If "mlpack"
provides a separate development package or SDK, be sure it has been installed.`

I checked in the mlpack files for the ones mentioned in the CMake error but couldn't find any of them.

I had already encountered this error during the APRIL installation and I then came across this issue (mlpack/mlpack#444) which seemed to indicate that the best option was to have the Findmlpack.cmake file.
As it solved the problem, I didn't really tried other ways of doing it since then

Where are you getting your mlpack from? It looks like it should be shipping a cmake configuration since v3.2.2 in principle. It might just be a question of pointing cmake into the right direction.

andresailer · 2024-08-28T13:46:58Z

CMakeLists.txt

+    IF( SDHCALContent_FOUND )
+        INCLUDE_DIRECTORIES( SYSTEM ${SDHCALContent_INCLUDE_DIRS} )
+        LINK_LIBRARIES( ${SDHCALContent_LIBRARIES} )
+        ADD_DEFINITIONS ( ${SDHCALContent_DEFINITIONS} )
+        ADD_DEFINITIONS( "-DSDHCALCONTENT" )
+    ENDIF()


Suggested change

IF( SDHCALContent_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${SDHCALContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${SDHCALContent_LIBRARIES} )

ADD_DEFINITIONS ( ${SDHCALContent_DEFINITIONS} )

ADD_DEFINITIONS( "-DSDHCALCONTENT" )

ENDIF()

IF( SDHCALContent_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${SDHCALContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${SDHCALContent_LIBRARIES} )

ADD_DEFINITIONS ( ${SDHCALContent_DEFINITIONS} )

ADD_DEFINITIONS( "SDHCALCONTENT" )

ENDIF()

cmake will add -D when needed, and in places where we have to treat definitions programatically it is easier if we don't have to deal with some with -D and some without.

andresailer · 2024-08-28T13:47:41Z

CMakeLists.txt

+    IF( APRILContent_FOUND )
+        INCLUDE_DIRECTORIES( SYSTEM ${APRILContent_INCLUDE_DIRS} )
+        LINK_LIBRARIES( ${APRILContent_LIBRARIES} )
+        ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )
+        ADD_DEFINITIONS( "-DAPRILCONTENT" )
+    ENDIF()
+    IF( mlpack_FOUND )
+        INCLUDE_DIRECTORIES( SYSTEM ${mlpack_INCLUDE_DIRS} )
+        LINK_LIBRARIES( ${mlpack_LIBRARIES} )
+        ADD_DEFINITIONS ( ${mlpack_DEFINITIONS} )
+    ENDIF()


Suggested change

IF( APRILContent_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${APRILContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${APRILContent_LIBRARIES} )

ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )

ADD_DEFINITIONS( "-DAPRILCONTENT" )

ENDIF()

IF( mlpack_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${mlpack_INCLUDE_DIRS} )

LINK_LIBRARIES( ${mlpack_LIBRARIES} )

ADD_DEFINITIONS ( ${mlpack_DEFINITIONS} )

ENDIF()

IF( APRILContent_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${APRILContent_INCLUDE_DIRS} )

LINK_LIBRARIES( ${APRILContent_LIBRARIES} )

ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )

ADD_DEFINITIONS( "APRILCONTENT" )

ENDIF()

IF( mlpack_FOUND )

INCLUDE_DIRECTORIES( SYSTEM ${mlpack_INCLUDE_DIRS} )

LINK_LIBRARIES( ${mlpack_LIBRARIES} )

ADD_DEFINITIONS ( ${mlpack_DEFINITIONS} )

ENDIF()

tmadlener · 2024-08-28T15:26:22Z

I think you will also have to rebase this onto the latest version of iLCSoft/DDMarlinPandora, as it looks like there is a merge conflict (which may resolve itself automatically during rebasing). Additionally, that will pick up the latest configuration for the CI.

…tent during compilation

…ation OK. 3 over 4 crash at runtime : commented out until cleaning in APRIL is done

…er 4 APRIL Registration OK

…ectly in the Marlin call without having to compile. Modified the README

…ts depending on the PFA that is used

…d particleId plugins from LCContent when using APRIL

tmadlener · 2024-09-23T13:15:26Z

cmake/Findmlpack.cmake

@tanguypasquier did you check, whether you can simply remove this file and things keep working?

tmadlener · 2024-09-23T13:18:11Z

CMakeLists.txt

+    IF( APRILContent_FOUND )
+        INCLUDE_DIRECTORIES( SYSTEM ${APRILContent_INCLUDE_DIRS} )
+        LINK_LIBRARIES( ${APRILContent_LIBRARIES} )
+        ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )
+        ADD_DEFINITIONS( "-DAPRILCONTENT" )
+    ENDIF()
+    IF( mlpack_FOUND )
+        INCLUDE_DIRECTORIES( SYSTEM ${mlpack_INCLUDE_DIRS} )
+        LINK_LIBRARIES( ${mlpack_LIBRARIES} )
+        ADD_DEFINITIONS ( ${mlpack_DEFINITIONS} )
+    ENDIF()


The special handling of adding -D might only be available for target_compile_definitions(?) @andresailer

tmadlener · 2024-09-23T13:19:50Z

CMakeLists.txt

+    FIND_PACKAGE( SDHCALContent REQUIRED )
+    INCLUDE_DIRECTORIES( SYSTEM ${SDHCALContent_INCLUDE_DIRS} )
+    LINK_LIBRARIES( ${SDHCALContent_LIBRARIES} )
+    ADD_DEFINITIONS ( ${SDHCALContent_DEFINITIONS} )


SDHCALContent doesn't seem to export any definitions(?): https://github.com/tanguypasquier/SDHCALContent/blob/main/cmake/SDHCALContentConfig.cmake.in

Suggested change

ADD_DEFINITIONS ( ${SDHCALContent_DEFINITIONS} )

tmadlener · 2024-09-23T13:20:42Z

CMakeLists.txt

+    FIND_PACKAGE( mlpack REQUIRED )
+    INCLUDE_DIRECTORIES( SYSTEM ${APRILContent_INCLUDE_DIRS} )
+    LINK_LIBRARIES( ${APRILContent_LIBRARIES} )
+    ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )


Suggested change

ADD_DEFINITIONS ( ${APRILContent_DEFINITIONS} )

APRILContent doesn't seem to export any definitions(?): https://github.com/SDHCAL/APRILContent/blob/current/cmake/APRILContentConfig.cmake.in

tmadlener · 2024-09-23T13:27:24Z

include/DDCaloHitCreator.h

+    /* DDCaloHitCreator& operator=(const DDCaloHitCreator&) = delete;  // Disallow copying
+    DDCaloHitCreator(const DDCaloHitCreator&) = delete; */


Suggested change

/* DDCaloHitCreator& operator=(const DDCaloHitCreator&) = delete; // Disallow copying

DDCaloHitCreator(const DDCaloHitCreator&) = delete; */

public:

DDCaloHitCreator& operator=(const DDCaloHitCreator&) = delete;

DDCaloHitCreator(const DDCaloHitCreator&) = delete;

Since you know have a unique_ptr member copying won't work in any case. Marking these as = delete will improve the compiler error message. (I would move them to the rest of the constructors a bit further above).

tmadlener reviewed Aug 28, 2024

View reviewed changes

andresailer reviewed Aug 28, 2024

View reviewed changes

andresailer mentioned this pull request Aug 28, 2024

Add possibility to include SDHCalContent and AprilContent #26

Closed

tanguypasquier and others added 18 commits September 20, 2024 11:03

made the changes to allow the inclusion of SDHCALContent and APRILCon…

a907286

…tent during compilation

Update README.md

47cb514

Update compilation instruction

ebd193d

First attempt to really include APRILContent : 1 over 4 APRIL registr…

08d484e

…ation OK. 3 over 4 crash at runtime : commented out until cleaning in APRIL is done

Cleaned APRIL and solved the problem for the RegisterAlgorithms. 2 ov…

0a2868f

…er 4 APRIL Registration OK

Added a parameter that allows to choose between Pandora and APRIL dir…

c070114

…ectly in the Marlin call without having to compile. Modified the README

Modified README and cleaned some comments

d6fc74c

Fixed the CaloHitFactory problem. Not fully optimized but works

df38b05

Created a new method that calls the right method to create the caloHi…

751ee26

…ts depending on the PFA that is used

Cleaned some comments

ea7a897

Fixed an issue with the Non-APRIL mode. Now works in all cases

55dd1a8

Deleted useless include

cdcf8f3

Included the choice of the factory instead of the last option

4557661

Created a function allowing the registration of energy corrections an…

76d15af

…d particleId plugins from LCContent when using APRIL

Removed unused include

fc937a7

Added some comments and modified README

bdc6e79

Update README.md

d1531bc

Update README.md

4e33c8c

tanguypasquier force-pushed the master branch from c37d689 to 4e33c8c Compare September 20, 2024 09:05

tanguypasquier added 3 commits September 20, 2024 11:21

CMakeLists updates

d49c7a0

Fixed Findmlpack.cmake

d8f1d2c

Fixed the factory issue

d793e70

tmadlener reviewed Sep 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

APRIL and SDHCALContent integration #29

APRIL and SDHCALContent integration #29

tanguypasquier commented Jul 10, 2024

tmadlener left a comment

tmadlener Aug 28, 2024

andresailer Aug 28, 2024

tanguypasquier Sep 20, 2024

tmadlener Sep 23, 2024

tmadlener Aug 28, 2024

andresailer Aug 28, 2024

tanguypasquier Sep 20, 2024

tmadlener Aug 28, 2024

tanguypasquier Sep 20, 2024

tmadlener Aug 28, 2024

tanguypasquier Sep 20, 2024

tmadlener Aug 28, 2024

tanguypasquier Sep 20, 2024

tmadlener Aug 28, 2024

tanguypasquier Sep 20, 2024

andresailer Aug 28, 2024

tmadlener Sep 23, 2024

tanguypasquier Sep 23, 2024

tmadlener Sep 23, 2024

andresailer Aug 28, 2024

andresailer Aug 28, 2024

tmadlener commented Aug 28, 2024

tmadlener Sep 23, 2024

tmadlener Sep 23, 2024

tmadlener Sep 23, 2024

tmadlener Sep 23, 2024

tmadlener Sep 23, 2024

	pandora::StatusCode PandoraHack(const pandora::Pandora &pandora);
	pandora::StatusCode AprilLCContentPluginRegistration(const pandora::Pandora &pandora);

	pandora::ObjectFactory<object_creation::CaloHit::Parameters, object_creation::CaloHit::Object>* caloHitFactory; //General factory to initialize
	pandora::ObjectFactory<object_creation::CaloHit::Parameters, object_creation::CaloHit::Object>* m_caloHitFactory; //General factory to initialize

		/* DDCaloHitCreator& operator=(const DDCaloHitCreator&) = delete; // Disallow copying
		DDCaloHitCreator(const DDCaloHitCreator&) = delete; */

APRIL and SDHCALContent integration #29

Are you sure you want to change the base?

APRIL and SDHCALContent integration #29

Conversation

tanguypasquier commented Jul 10, 2024

tmadlener left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmadlener commented Aug 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment