Skip to content

Commit

Permalink
Fix JANA2-related deprecation warnings and TODOs (#1622)
Browse files Browse the repository at this point in the history
### Briefly, what does this PR introduce?

This cleans up the EICrecon compilation output by fixing some
deprecation warnings coming from JANA2. It also removes some
JANA2-related TODO comments. It updates a test case to no longer use a
template method that hasn't been deprecated, but is about to be.

### What kind of change does this PR introduce?
- [ ] Bug fix (issue #__)
- [ ] New feature (issue #__)
- [ ] Documentation update
- [x] Other: Fixes deprecation warnings

### Please check if this PR fulfills the following:
- [ ] Tests for the changes have been added
- [ ] Documentation has been added / updated
- [ ] Changes have been communicated to collaborators

### Does this PR introduce breaking changes? What changes might users
need to make to their code?
No

### Does this PR change default behavior?
No
  • Loading branch information
nathanwbrei authored Sep 21, 2024
1 parent ae5ce86 commit b2a846e
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 14 deletions.
5 changes: 1 addition & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,10 @@ set(CMAKE_INSTALL_RPATH
set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)

# Check and print what JANA2 is used
find_package(JANA 2.0.8 REQUIRED)
find_package(JANA REQUIRED)
message(STATUS "${CMAKE_PROJECT_NAME}: JANA2 CMake : ${JANA_DIR}")
message(STATUS "${CMAKE_PROJECT_NAME}: JANA2 includes: ${JANA_INCLUDE_DIR}")
message(STATUS "${CMAKE_PROJECT_NAME}: JANA2 library : ${JANA_LIBRARY}")
add_compile_definitions(HAVE_PODIO)
# TODO: NWB: Do this correctly in JANA via target_compile_definitions for v2.1.1
# TODO: NWB: Maybe we want these to be prefixed, e.g. JANA2_HAVE_PODIO

# Algorithms
find_package(algorithms 1.0.0 REQUIRED Core)
Expand Down
1 change: 0 additions & 1 deletion src/extensions/jana/JOmniFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,6 @@ class JOmniFactory : public JMultifactory {
std::vector<std::string> default_input_collection_names,
std::vector<std::string> default_output_collection_names ) {

// TODO: NWB: JMultiFactory::GetTag,SetTag are not currently usable
m_prefix = (this->GetPluginName().empty()) ? tag : this->GetPluginName() + ":" + tag;

// Obtain collection name overrides if provided.
Expand Down
3 changes: 0 additions & 3 deletions src/extensions/jana/JOmniFactoryGeneratorT.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,6 @@ class JOmniFactoryGeneratorT : public JFactoryGenerator {
factory->SetApplication(m_app);
factory->SetPluginName(this->GetPluginName());
factory->SetFactoryName(JTypeInfo::demangle<FactoryT>());
// factory->SetTag(wiring.m_tag);
// We do NOT want to do this because JMF will use the tag to suffix the collection names
// TODO: NWB: Change this in JANA
factory->config() = wiring.m_default_cfg;

// Set up all of the wiring prereqs so that Init() can do its thing
Expand Down
13 changes: 7 additions & 6 deletions src/tests/omnifactory_test/JOmniFactoryTests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <JANA/JMultifactory.h>
#include <JANA/Services/JComponentManager.h>
#include <JANA/Services/JParameterManager.h>
#include <JANA/Utils/JTypeInfo.h>
#include <catch2/catch_test_macros.hpp>
#include <edm4hep/SimCalorimeterHitCollection.h>
#include <fmt/core.h>
Expand Down Expand Up @@ -55,7 +56,6 @@ struct BasicTestAlg : public JOmniFactory<BasicTestAlg, BasicTestAlgConfig> {
m_process_call_count++;
logger()->info("Calling BasicTestAlg::Process with bucket_count={}, threshold={}", config().bucket_count, config().threshold);
// Provide empty collections (as opposed to nulls) so that PODIO doesn't crash
// TODO: NWB: I though multifactories already took care of this under the hood somewhere
output_hits_left() = std::make_unique<edm4hep::SimCalorimeterHitCollection>();
output_hits_right() = std::make_unique<edm4hep::SimCalorimeterHitCollection>();
output_vechits().push_back(new edm4hep::SimCalorimeterHit());
Expand All @@ -64,7 +64,7 @@ struct BasicTestAlg : public JOmniFactory<BasicTestAlg, BasicTestAlgConfig> {

template <typename OutputCollectionT, typename MultifactoryT>
MultifactoryT* RetrieveMultifactory(JFactorySet* facset, std::string output_collection_name) {
auto fac = facset->GetFactory<OutputCollectionT>(output_collection_name);
auto fac = facset->GetFactory(JTypeInfo::demangle<OutputCollectionT>(), output_collection_name);
REQUIRE(fac != nullptr);
auto helper = dynamic_cast<JMultifactoryHelperPodio<OutputCollectionT>*>(fac);
REQUIRE(helper != nullptr);
Expand Down Expand Up @@ -180,10 +180,11 @@ TEST_CASE("JParameterManager correctly understands which values are defaulted an
REQUIRE(b->config().threshold == 12.0);

std::cout << "Showing the full table of config parameters" << std::endl;
app.GetJParameterManager()->PrintParameters(true, false, true);
app.GetJParameterManager()->PrintParameters(2, 1); // verbosity, strictness

std::cout << "Showing only overridden config parameters" << std::endl;
app.GetJParameterManager()->PrintParameters(false, false, true);
app.GetJParameterManager()->PrintParameters(1, 1); // verbosity, strictness

}

TEST_CASE("Wiring itself is correctly defaulted") {
Expand Down Expand Up @@ -220,11 +221,11 @@ TEST_CASE("Wiring itself is correctly defaulted") {


b->logger()->info("Showing the full table of config parameters");
app.GetJParameterManager()->PrintParameters(true, false, true);
app.GetJParameterManager()->PrintParameters(2,1); // verbosity, strictness

b->logger()->info("Showing only overridden config parameters");
// Should be empty because everything is defaulted
app.GetJParameterManager()->PrintParameters(false, false, true);
app.GetJParameterManager()->PrintParameters(1,1); // verbosity, strictness
}

struct VariadicTestAlg : public JOmniFactory<VariadicTestAlg, BasicTestAlgConfig> {
Expand Down

0 comments on commit b2a846e

Please sign in to comment.