From 89245c3df52192595b21453aa9cb933427dcb078 Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Wed, 25 Jan 2023 11:46:01 -0700 Subject: [PATCH] Remove NUMA regions option This configuration option was only added because Kokkos has such a flag. But this flag is now deprecated in Kokkos and has no effect, so remove it from VTK-m. --- vtkm/cont/RuntimeDeviceInformation.cxx | 14 ------------ .../internal/RuntimeDeviceConfiguration.cxx | 15 ------------- .../internal/RuntimeDeviceConfiguration.h | 2 -- .../RuntimeDeviceConfigurationOptions.cxx | 16 ++++++-------- .../RuntimeDeviceConfigurationOptions.h | 1 - .../RuntimeDeviceConfigurationKokkos.h | 21 ------------------ ...itTestKokkosRuntimeDeviceConfiguration.cxx | 21 ++++-------------- .../TestingRuntimeDeviceConfiguration.h | 1 - vtkm/cont/testing/UnitTestInitialize.cxx | 12 ++-------- .../UnitTestRuntimeConfigurationOptions.cxx | 22 ++++--------------- 10 files changed, 17 insertions(+), 108 deletions(-) diff --git a/vtkm/cont/RuntimeDeviceInformation.cxx b/vtkm/cont/RuntimeDeviceInformation.cxx index 0efc5b9a6..f8e95eb3b 100644 --- a/vtkm/cont/RuntimeDeviceInformation.cxx +++ b/vtkm/cont/RuntimeDeviceInformation.cxx @@ -97,13 +97,6 @@ public: throw vtkm::cont::ErrorBadDevice("Tried to set the number of threads on an invalid device"); } - VTKM_CONT virtual vtkm::cont::internal::RuntimeDeviceConfigReturnCode SetNumaRegions( - const vtkm::Id&) override final - { - throw vtkm::cont::ErrorBadDevice( - "Tried to set the number of numa regions on an invalid device"); - } - VTKM_CONT virtual vtkm::cont::internal::RuntimeDeviceConfigReturnCode SetDeviceInstance( const vtkm::Id&) override final { @@ -116,13 +109,6 @@ public: throw vtkm::cont::ErrorBadDevice("Tried to get the number of threads on an invalid device"); } - VTKM_CONT virtual vtkm::cont::internal::RuntimeDeviceConfigReturnCode GetNumaRegions( - vtkm::Id&) const override final - { - throw vtkm::cont::ErrorBadDevice( - "Tried to get the number of numa regions on an invalid device"); - } - VTKM_CONT virtual vtkm::cont::internal::RuntimeDeviceConfigReturnCode GetDeviceInstance( vtkm::Id&) const override final { diff --git a/vtkm/cont/internal/RuntimeDeviceConfiguration.cxx b/vtkm/cont/internal/RuntimeDeviceConfiguration.cxx index 8569bd71d..5b4f0b37d 100644 --- a/vtkm/cont/internal/RuntimeDeviceConfiguration.cxx +++ b/vtkm/cont/internal/RuntimeDeviceConfiguration.cxx @@ -90,11 +90,6 @@ void RuntimeDeviceConfigurationBase::Initialize( [&](const vtkm::Id& value) { return this->SetThreads(value); }, "SetThreads", this->GetDevice().GetName()); - InitializeOption( - configOptions.VTKmNumaRegions, - [&](const vtkm::Id& value) { return this->SetNumaRegions(value); }, - "SetNumaRegions", - this->GetDevice().GetName()); InitializeOption( configOptions.VTKmDeviceInstance, [&](const vtkm::Id& value) { return this->SetDeviceInstance(value); }, @@ -117,11 +112,6 @@ RuntimeDeviceConfigReturnCode RuntimeDeviceConfigurationBase::SetThreads(const v return RuntimeDeviceConfigReturnCode::INVALID_FOR_DEVICE; } -RuntimeDeviceConfigReturnCode RuntimeDeviceConfigurationBase::SetNumaRegions(const vtkm::Id&) -{ - return RuntimeDeviceConfigReturnCode::INVALID_FOR_DEVICE; -} - RuntimeDeviceConfigReturnCode RuntimeDeviceConfigurationBase::SetDeviceInstance(const vtkm::Id&) { return RuntimeDeviceConfigReturnCode::INVALID_FOR_DEVICE; @@ -132,11 +122,6 @@ RuntimeDeviceConfigReturnCode RuntimeDeviceConfigurationBase::GetThreads(vtkm::I return RuntimeDeviceConfigReturnCode::INVALID_FOR_DEVICE; } -RuntimeDeviceConfigReturnCode RuntimeDeviceConfigurationBase::GetNumaRegions(vtkm::Id&) const -{ - return RuntimeDeviceConfigReturnCode::INVALID_FOR_DEVICE; -} - RuntimeDeviceConfigReturnCode RuntimeDeviceConfigurationBase::GetDeviceInstance(vtkm::Id&) const { return RuntimeDeviceConfigReturnCode::INVALID_FOR_DEVICE; diff --git a/vtkm/cont/internal/RuntimeDeviceConfiguration.h b/vtkm/cont/internal/RuntimeDeviceConfiguration.h index f71623d3d..a51d7698c 100644 --- a/vtkm/cont/internal/RuntimeDeviceConfiguration.h +++ b/vtkm/cont/internal/RuntimeDeviceConfiguration.h @@ -54,13 +54,11 @@ public: /// A method should return INVALID_FOR_DEVICE if the overriden device does not /// support the particular set method. VTKM_CONT virtual RuntimeDeviceConfigReturnCode SetThreads(const vtkm::Id& value); - VTKM_CONT virtual RuntimeDeviceConfigReturnCode SetNumaRegions(const vtkm::Id& value); VTKM_CONT virtual RuntimeDeviceConfigReturnCode SetDeviceInstance(const vtkm::Id& value); /// The following public methods are overriden in each individual device and store the /// values that were set via the above Set* methods for the given device. VTKM_CONT virtual RuntimeDeviceConfigReturnCode GetThreads(vtkm::Id& value) const; - VTKM_CONT virtual RuntimeDeviceConfigReturnCode GetNumaRegions(vtkm::Id& value) const; VTKM_CONT virtual RuntimeDeviceConfigReturnCode GetDeviceInstance(vtkm::Id& value) const; /// The following public methods should be overriden as needed for each individual device diff --git a/vtkm/cont/internal/RuntimeDeviceConfigurationOptions.cxx b/vtkm/cont/internal/RuntimeDeviceConfigurationOptions.cxx index 7ebb9c214..ccea11c81 100644 --- a/vtkm/cont/internal/RuntimeDeviceConfigurationOptions.cxx +++ b/vtkm/cont/internal/RuntimeDeviceConfigurationOptions.cxx @@ -31,13 +31,13 @@ void AppendOptionDescriptors(std::vector& usage, "vtkm-num-threads", option::VtkmArg::Required, " --vtkm-num-threads \tSets the number of threads to use for the selected device" }); - usage.push_back( - { useOptionIndex ? static_cast(option::OptionIndex::NUMA_REGIONS) : 1, - 0, - "", - "vtkm-numa-regions", - option::VtkmArg::Required, - " --vtkm-numa-regions \tSets the number of numa regions when using kokkos/OpenMP" }); + usage.push_back({ useOptionIndex ? static_cast(option::OptionIndex::NUMA_REGIONS) : 1, + 0, + "", + "vtkm-numa-regions", + option::VtkmArg::Required, + " --vtkm-numa-regions \tSets the number of numa regions when using " + "kokkos/OpenMP (deprecated, has no effect)" }); usage.push_back( { useOptionIndex ? static_cast(option::OptionIndex::DEVICE_INSTANCE) : 2, 0, @@ -51,7 +51,6 @@ void AppendOptionDescriptors(std::vector& usage, RuntimeDeviceConfigurationOptions::RuntimeDeviceConfigurationOptions(const bool& useOptionIndex) : VTKmNumThreads(useOptionIndex ? option::OptionIndex::NUM_THREADS : 0, "VTKM_NUM_THREADS") - , VTKmNumaRegions(useOptionIndex ? option::OptionIndex::NUMA_REGIONS : 1, "VTKM_NUMA_REGIONS") , VTKmDeviceInstance(useOptionIndex ? option::OptionIndex::DEVICE_INSTANCE : 2, "VTKM_DEVICE_INSTANCE") , Initialized(false) @@ -100,7 +99,6 @@ RuntimeDeviceConfigurationOptions::~RuntimeDeviceConfigurationOptions() noexcept void RuntimeDeviceConfigurationOptions::Initialize(const option::Option* options) { this->VTKmNumThreads.Initialize(options); - this->VTKmNumaRegions.Initialize(options); this->VTKmDeviceInstance.Initialize(options); this->Initialized = true; } diff --git a/vtkm/cont/internal/RuntimeDeviceConfigurationOptions.h b/vtkm/cont/internal/RuntimeDeviceConfigurationOptions.h index 619093b59..4c17c8845 100644 --- a/vtkm/cont/internal/RuntimeDeviceConfigurationOptions.h +++ b/vtkm/cont/internal/RuntimeDeviceConfigurationOptions.h @@ -48,7 +48,6 @@ public: VTKM_CONT bool IsInitialized() const; RuntimeDeviceOption VTKmNumThreads; - RuntimeDeviceOption VTKmNumaRegions; RuntimeDeviceOption VTKmDeviceInstance; protected: diff --git a/vtkm/cont/kokkos/internal/RuntimeDeviceConfigurationKokkos.h b/vtkm/cont/kokkos/internal/RuntimeDeviceConfigurationKokkos.h index 4293b8b6b..9a22dfab8 100644 --- a/vtkm/cont/kokkos/internal/RuntimeDeviceConfigurationKokkos.h +++ b/vtkm/cont/kokkos/internal/RuntimeDeviceConfigurationKokkos.h @@ -99,21 +99,6 @@ public: return RuntimeDeviceConfigReturnCode::SUCCESS; } - VTKM_CONT virtual RuntimeDeviceConfigReturnCode SetNumaRegions( - const vtkm::Id& value) override final - { - if (Kokkos::is_initialized()) - { - VTKM_LOG_S(vtkm::cont::LogLevel::Warn, - "SetNumaRegions was called but Kokkos was already initailized! Updates will not " - "be applied."); - return RuntimeDeviceConfigReturnCode::NOT_APPLIED; - } - this->KokkosArguments.insert(this->KokkosArguments.begin(), - "--kokkos-numa=" + std::to_string(value)); - return RuntimeDeviceConfigReturnCode::SUCCESS; - } - VTKM_CONT virtual RuntimeDeviceConfigReturnCode SetDeviceInstance( const vtkm::Id& value) override final { @@ -134,12 +119,6 @@ public: return GetArgFromList(this->KokkosArguments, "--kokkos-num-threads", value); } - VTKM_CONT virtual RuntimeDeviceConfigReturnCode GetNumaRegions( - vtkm::Id& value) const override final - { - return GetArgFromList(this->KokkosArguments, "--kokkos-numa", value); - } - VTKM_CONT virtual RuntimeDeviceConfigReturnCode GetDeviceInstance( vtkm::Id& value) const override final { diff --git a/vtkm/cont/kokkos/testing/UnitTestKokkosRuntimeDeviceConfiguration.cxx b/vtkm/cont/kokkos/testing/UnitTestKokkosRuntimeDeviceConfiguration.cxx index afe45adc1..0e80423c6 100644 --- a/vtkm/cont/kokkos/testing/UnitTestKokkosRuntimeDeviceConfiguration.cxx +++ b/vtkm/cont/kokkos/testing/UnitTestKokkosRuntimeDeviceConfiguration.cxx @@ -40,28 +40,21 @@ TestingRuntimeDeviceConfiguration::TestRunti "Failed to get set threads"); VTKM_TEST_ASSERT(testValue == 8, "Set threads does not match expected value: 8 != " + std::to_string(testValue)); - VTKM_TEST_ASSERT(config.GetNumaRegions(testValue) == - internal::RuntimeDeviceConfigReturnCode::SUCCESS, - "Failed to get set numa regions"); - VTKM_TEST_ASSERT(testValue == 4, - "Set numa regions does not match expected value: 4 != " + - std::to_string(testValue)); VTKM_TEST_ASSERT(config.GetDeviceInstance(testValue) == internal::RuntimeDeviceConfigReturnCode::SUCCESS, "Failed to get set device instance"); VTKM_TEST_ASSERT(testValue == 0, "Set device instance does not match expected value: 0 != " + std::to_string(testValue)); - // Ensure that with kokkos we can't re-initialize or set values after the first initialize - // Should pop up a few warnings in the test logs + std::cout + << "Ensure that with kokkos we can't re-initialize or set values after the first initialize" + << std::endl; + std::cout << "This should pop up a few warnings in the test logs" << std::endl; deviceOptions.VTKmNumThreads.SetOption(16); - deviceOptions.VTKmNumaRegions.SetOption(2); deviceOptions.VTKmDeviceInstance.SetOption(5); config.Initialize(deviceOptions); VTKM_TEST_ASSERT(config.SetThreads(1) == internal::RuntimeDeviceConfigReturnCode::NOT_APPLIED, "Shouldn't be able to set threads after kokkos is initalized"); - VTKM_TEST_ASSERT(config.SetNumaRegions(1) == internal::RuntimeDeviceConfigReturnCode::NOT_APPLIED, - "Shouldn't be able to set numa regions after kokkos is initalized"); VTKM_TEST_ASSERT(config.SetDeviceInstance(1) == internal::RuntimeDeviceConfigReturnCode::NOT_APPLIED, "Shouldn't be able to set device instnace after kokkos is initalized"); @@ -71,12 +64,6 @@ TestingRuntimeDeviceConfiguration::TestRunti "Failed to get set threads"); VTKM_TEST_ASSERT(testValue == 8, "Set threads does not match expected value: 8 != " + std::to_string(testValue)); - VTKM_TEST_ASSERT(config.GetNumaRegions(testValue) == - internal::RuntimeDeviceConfigReturnCode::SUCCESS, - "Failed to get set numa regions"); - VTKM_TEST_ASSERT(testValue == 4, - "Set numa regions does not match expected value: 4 != " + - std::to_string(testValue)); VTKM_TEST_ASSERT(config.GetDeviceInstance(testValue) == internal::RuntimeDeviceConfigReturnCode::SUCCESS, "Failed to get set device instance"); diff --git a/vtkm/cont/testing/TestingRuntimeDeviceConfiguration.h b/vtkm/cont/testing/TestingRuntimeDeviceConfiguration.h index 6d5677289..20a1bff53 100644 --- a/vtkm/cont/testing/TestingRuntimeDeviceConfiguration.h +++ b/vtkm/cont/testing/TestingRuntimeDeviceConfiguration.h @@ -35,7 +35,6 @@ struct TestingRuntimeDeviceConfiguration { internal::RuntimeDeviceConfigurationOptions runtimeDeviceOptions{}; runtimeDeviceOptions.VTKmNumThreads.SetOption(8); - runtimeDeviceOptions.VTKmNumaRegions.SetOption(0); runtimeDeviceOptions.VTKmDeviceInstance.SetOption(2); runtimeDeviceOptions.Initialize(nullptr); VTKM_TEST_ASSERT(runtimeDeviceOptions.IsInitialized(), diff --git a/vtkm/cont/testing/UnitTestInitialize.cxx b/vtkm/cont/testing/UnitTestInitialize.cxx index 06f3ef0a4..961fd1f24 100644 --- a/vtkm/cont/testing/UnitTestInitialize.cxx +++ b/vtkm/cont/testing/UnitTestInitialize.cxx @@ -146,16 +146,8 @@ void InitializeRuntimeDeviceConfigurationWithArgs() { int argc; char** argv; - vtkm::cont::testing::Testing::MakeArgsAddProgramName(argc, - argv, - "--vtkm-device", - "Any", - "--vtkm-num-threads", - "100", - "--vtkm-numa-regions", - "4", - "--vtkm-device-instance", - "2"); + vtkm::cont::testing::Testing::MakeArgsAddProgramName( + argc, argv, "--vtkm-device", "Any", "--vtkm-num-threads", "100", "--vtkm-device-instance", "2"); vtkm::cont::Initialize(argc, argv); CheckArgs(argc, argv); } diff --git a/vtkm/cont/testing/UnitTestRuntimeConfigurationOptions.cxx b/vtkm/cont/testing/UnitTestRuntimeConfigurationOptions.cxx index f13bf1139..0d1bee136 100644 --- a/vtkm/cont/testing/UnitTestRuntimeConfigurationOptions.cxx +++ b/vtkm/cont/testing/UnitTestRuntimeConfigurationOptions.cxx @@ -187,11 +187,9 @@ void TestConfigOptionValues(const internal::RuntimeDeviceConfigurationOptions& c VTKM_TEST_ASSERT(configOptions.IsInitialized(), "runtime config options should be initialized"); VTKM_TEST_ASSERT(configOptions.VTKmNumThreads.IsSet(), "num threads should be set"); - VTKM_TEST_ASSERT(configOptions.VTKmNumaRegions.IsSet(), "numa regions should be set"); VTKM_TEST_ASSERT(configOptions.VTKmDeviceInstance.IsSet(), "device instance should be set"); VTKM_TEST_ASSERT(configOptions.VTKmNumThreads.GetValue() == 100, "num threads should == 100"); - VTKM_TEST_ASSERT(configOptions.VTKmNumaRegions.GetValue() == 2, "numa regions should == 2"); VTKM_TEST_ASSERT(configOptions.VTKmDeviceInstance.GetValue() == 1, "device instance should == 1"); } @@ -211,14 +209,8 @@ void TestRuntimeDeviceConfigurationOptions() int argc; char** argv; - vtkm::cont::testing::Testing::MakeArgs(argc, - argv, - "--vtkm-num-threads", - "100", - "--vtkm-numa-regions", - "2", - "--vtkm-device-instance", - "1"); + vtkm::cont::testing::Testing::MakeArgs( + argc, argv, "--vtkm-num-threads", "100", "--vtkm-device-instance", "1"); auto options = GetOptions(argc, argv, usage); VTKM_TEST_ASSERT(!configOptions.IsInitialized(), @@ -230,14 +222,8 @@ void TestRuntimeDeviceConfigurationOptions() { int argc; char** argv; - vtkm::cont::testing::Testing::MakeArgs(argc, - argv, - "--vtkm-num-threads", - "100", - "--vtkm-numa-regions", - "2", - "--vtkm-device-instance", - "1"); + vtkm::cont::testing::Testing::MakeArgs( + argc, argv, "--vtkm-num-threads", "100", "--vtkm-device-instance", "1"); internal::RuntimeDeviceConfigurationOptions configOptions(argc, argv); TestConfigOptionValues(configOptions); }