Variorium Connector: kernel granularity and JSON #265

twilk10 · 2024-07-19T19:17:28Z

Updates to the current connector with updates such as json updates, energy estimation, device information. It has also been altered to get power information at the kernel level.

masterleinad · 2024-07-19T19:47:02Z

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

masterleinad

Please provide sample output.

masterleinad · 2024-07-19T19:48:09Z

profiling/variorum-connector/variorum-connector.cpp

-#include <inttypes.h>
+#include <chrono>
 #include <cstdlib>
 #include <cstring>
-#include <vector>
-#include <unordered_set>
-#include <string>
-#include <regex>
+#include <ctime>
 #include <cxxabi.h>
 #include <dlfcn.h>
-#include <ctime>
-#include <chrono>
-#include <iostream>
 #include <fstream>
-
+#include <inttypes.h>
+#include <iostream>
+#include <regex>
+#include <stdio.h>
+#include <string>
+#include <unordered_set>
+#include <vector>


Which of these header files are actually required? Please make sure to only use what you need.

masterleinad · 2024-07-19T19:49:18Z

profiling/variorum-connector/variorum-connector.cpp

+int power = -1;
+int power1 = 0;
+int power2 = 0;
+int filemake = -1;
+long long time1 = 0;
+long long time2 = 0;
+uint32_t gdevID = -1;
+std::vector<float> gpu_powers;
+std::vector<float> gpu_powers2;
+std::string output = "";


What are all these global variables used for? Please add comments in the code explaining their usage.

masterleinad · 2024-07-19T19:51:16Z

profiling/variorum-connector/variorum-connector.cpp

 int type_of_profiling =
-    0;  // 0 is for both print power & json, 1 is for print power, 2 is for json
-bool usingMPI     = false;
-bool verbosePrint = false;
-bool mpiOutPut    = false;
+    0; // 0 is for both print power & json, 1 is for print power, 2 is for json
+bool usingMPI = false;
+bool verbosePrint = true;
+bool mpiOutPut = false;


Are all of these still used/necessary?

masterleinad · 2024-07-19T19:51:34Z

profiling/variorum-connector/variorum-connector.cpp

+/*void printFile() {
+
+    std::ofstream file("variorumoutput.txt", std::ios::app); // Open in append
+mode if (!file) { std::cerr << "Error creating the file!" << std::endl; } else {
+        std::cout << "File created or opened successfully." << std::endl;
+    }
+    file.close(); // Close the file
+i}*/


Please remove unused code.

masterleinad · 2024-07-19T19:52:12Z

profiling/variorum-connector/variorum-connector.cpp

+    file.close(); // Close the file
+i}*/
+void printFile() {
+  const std::string filename = "variorumoutput.txt";


Maybe make this configurable via an environment variable.

masterleinad · 2024-07-19T20:05:03Z

profiling/variorum-connector/variorum-connector.cpp

+        //          std::cout << "Number of Sockets: " << num_sockets <<
+        //          std::endl;


Suggested change

// std::cout << "Number of Sockets: " << num_sockets <<

// std::endl;

masterleinad · 2024-07-19T20:05:38Z

profiling/variorum-connector/variorum-connector.cpp

+    char *num_sockets_pos = strstr(s, "\"num_gpus_per_socket\":");
+    if (num_sockets_pos != nullptr) {
+      num_sockets_pos += strlen("\"num_gpus_per_socket\":");
+
+      char *num_sockets_end_pos = strchr(num_sockets_pos, ',');
+      if (num_sockets_end_pos == nullptr) {
+        num_sockets_end_pos = strchr(num_sockets_pos, '}');
+      }
+      if (num_sockets_end_pos != nullptr) {
+
+        std::string num_sockets_str(num_sockets_pos,
+                                    num_sockets_end_pos - num_sockets_pos);
+
+        num_sockets = std::stoll(num_sockets_str);


Please try using jansson here as well (and below).

masterleinad · 2024-07-19T20:06:25Z

profiling/variorum-connector/variorum-connector.cpp

+      std::cout << "  Gpu start power: " << gpu_powers2[gdevID]
+                << " Gpu end power: " << gpu_powers[gdevID]
+                << "   Energy Estimation:   "
+                << ((gpu_powers[gdevID] + gpu_powers2[0]) / 2) *
+                       ((temp - time1) * .001)
+                << std::endl;


Let's just not print anything to stdout but only to the file.

Sounds fine to me.

masterleinad · 2024-07-19T20:07:06Z

profiling/variorum-connector/variorum-connector.cpp

+  /*	std::cout << "kokkos library call\n" << std::endl;
+    if (usingMPI) {
+      variorum_call_mpi();
+    } else {
+     variorum_call();
+    }
+    time_t total_time;
+    time_t end_time;
+    time(&end_time);
+    std::cout << "End Time: " << end_time << "\nStart Time: " << start_time
+              << "\n";
+    total_time = end_time - start_time;
+
+    std::cout << "The kokkos library was alive for " << total_time << "
+    seconds."
+              << std::endl;*/


Suggested change

/* std::cout << "kokkos library call\n" << std::endl;

if (usingMPI) {

variorum_call_mpi();

} else {

variorum_call();

}

time_t total_time;

time_t end_time;

time(&end_time);

std::cout << "End Time: " << end_time << "\nStart Time: " << start_time

<< "\n";

total_time = end_time - start_time;

std::cout << "The kokkos library was alive for " << total_time << "

seconds."

<< std::endl;*/

@twilk10 Is this suggestion OK with you? If so, can you commit it?

It is good with me.

masterleinad · 2024-07-19T20:10:35Z

profiling/variorum-connector/variorum-connector.cpp

+std::ostream &operator<<(
+    std::ostream &os,
+    const Kokkos::Tools::Experimental::ExecutionSpaceIdentifier &identifier) {
+
+  os << "  Type: " << identifier.type << "";
+  os << "  Device ID: " << identifier.device_id << "";
+  gdevID = identifier.device_id;
+  os << "  Instance ID: " << identifier.instance_id;
+  //   output += " Device ID: " + std::to_string(identifier.device_id) +
+  //                      " Instance ID: " +
+  //                      std::to_string(identifier.instance_id);
+  writeToFile("variorumoutput.txt", output);
+  return os;
+}


Don't overload operator<< but just write to the output file directly.

masterleinad · 2024-07-19T20:12:28Z

Please use clang-format 8 for the indentation.

vlkale · 2024-07-19T20:21:16Z

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1

@twilk10 Thanks for submitting this PR!

To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

twilk10 · 2024-07-19T22:38:41Z

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1

@twilk10 Thanks for submitting this PR!

To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

Thank you I will work on getting that information up as soon as possible.

vlkale · 2024-07-20T02:23:16Z

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1
@twilk10 Thanks for submitting this PR!
To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

Thank you I will work on getting that information up as soon as possible.

Thanks! Be sure to run clang-format-8 on each code file that you changed in this PR, as @masterleinad mentions in one of his comments. This is a prereq for your PR to be merged. Note that this is the CI check that doesn't pass.

twilk10 · 2024-07-20T03:00:43Z

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1
@twilk10 Thanks for submitting this PR!
To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

Thank you I will work on getting that information up as soon as possible.

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1
@twilk10 Thanks for submitting this PR!
To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

Thank you I will work on getting that information up as soon as possible.

Thanks! Be sure to run clang-format-8 on each code file that you changed in this PR, as @masterleinad mentions in one of his comments. This is a prereq for your PR to be merged. Note that this is the CI check that doesn't pass.

I ran clang-format 8 on it I will have to evaluate what went wrong and try it again.

vlkale · 2024-07-23T14:27:32Z

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1
@twilk10 Thanks for submitting this PR!
To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

Thank you I will work on getting that information up as soon as possible.

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1
@twilk10 Thanks for submitting this PR!
To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

Thank you I will work on getting that information up as soon as possible.

Thanks! Be sure to run clang-format-8 on each code file that you changed in this PR, as @masterleinad mentions in one of his comments. This is a prereq for your PR to be merged. Note that this is the CI check that doesn't pass.

I ran clang-format 8 on it I will have to evaluate what went wrong and try it again.

@twilk10 OK. You want to do something like:

clang-format-8 -style=file --assume-filename=../../.clang-format ../../profiling/variorium-connector/kp_variorium_connector.cpp > ../../profiling/variorium-connector/kp_variorium_connector-temp.cpp

You can diff in between to see whether and/or how the file has indeed changed and then mv the temporary file to the original file.

If that doesn't work, can you send the error output with the flag -Werror ?

vlkale · 2024-07-23T14:33:07Z

@twilk10 Can you look at @masterleinad 's comments particularly on printing output and parsing json with jansson? I think they are good suggestions and maybe you can commit those before committing other changes with clang-format.

Also, make sure to note to not include any unneeded headers.

twilk10 · 2024-07-23T14:38:33Z

@twilk10 Can you look at @masterleinad 's comments particularly on printing output and parsing json with jansson? I think they are good suggestions and maybe you can commit those before committing other changes with clang-format.

Also, make sure to note to not include any unneeded headers.

I will take a look and try updating withthe help of these suggestions.

twilk10 · 2024-07-23T14:45:28Z

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1
@twilk10 Thanks for submitting this PR!
To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

Thank you I will work on getting that information up as soon as possible.

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1
@twilk10 Thanks for submitting this PR!
To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

Thank you I will work on getting that information up as soon as possible.

Thanks! Be sure to run clang-format-8 on each code file that you changed in this PR, as @masterleinad mentions in one of his comments. This is a prereq for your PR to be merged. Note that this is the CI check that doesn't pass.

I ran clang-format 8 on it I will have to evaluate what went wrong and try it again.

@twilk10 OK. You want to do something like:

clang-format-8 -style=file --assume-filename=../../.clang-format ../../profiling/variorium-connector/kp_variorium_connector.cpp > ../../profiling/variorium-connector/kp_variorium_connector-temp.cpp

You can diff in between to see whether and/or how the file has indeed changed and then mv the temporary file to the original file.

If that doesn't work, can you send the error output with the flag -Werror ?

I am having trouble getting access to clang-format-8 but I can get other versions.

twilk10 · 2024-07-23T16:44:13Z

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1
@twilk10 Thanks for submitting this PR!
To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

Thank you I will work on getting that information up as soon as possible.

Please explain the behavior of the connector tool including options. What is the minimum variorum version required? We should check for that when trying to find Variorum via CMake.

+1
@twilk10 Thanks for submitting this PR!
To provide an explanation of this connector tool’s options and a high-level behavior from the Kokkos user perspective: maybe you can add kokkosp_print_help() (like what is in space-time-stack).

Thank you I will work on getting that information up as soon as possible.

Thanks! Be sure to run clang-format-8 on each code file that you changed in this PR, as @masterleinad mentions in one of his comments. This is a prereq for your PR to be merged. Note that this is the CI check that doesn't pass.

I ran clang-format 8 on it I will have to evaluate what went wrong and try it again.

@twilk10 OK. You want to do something like:
clang-format-8 -style=file --assume-filename=../../.clang-format ../../profiling/variorium-connector/kp_variorium_connector.cpp > ../../profiling/variorium-connector/kp_variorium_connector-temp.cpp
You can diff in between to see whether and/or how the file has indeed changed and then mv the temporary file to the original file.
If that doesn't work, can you send the error output with the flag -Werror ?

I am having trouble getting access to clang-format-8 but I can get other versions.

I have solved this issue.

vlkale · 2024-07-23T19:02:21Z

Great! All checks have passed for clang-format-8. Can you address the comments by @masterleinad ? (You can commit suggestions if you agree).

I also updated the title to capture the changes you made in your code better.

@masterleinad Are there any other things to resolve here?

twilk10 · 2024-07-23T19:11:04Z

Great! All checks have passed for clang-format-8. Can you address the comments by @masterleinad ? (You can commit suggestions if you agree).

I also updated the title to capture the changes you made in your code better.

@masterleinad Are there any other things to resolve here?

I am currently working on updating the connector to address the comments made by @masterleinad

masterleinad · 2024-07-23T19:39:07Z

profiling/variorum-connector/variorum-connector.cpp

@@ -1,4 +1,5 @@
-//@HEADER
+//@HEADER140


Suggested change

//@HEADER140

//@HEADER

You still have HEADER140 here.

masterleinad · 2024-07-23T19:45:47Z

profiling/variorum-connector/variorum-connector.cpp

+std::ostream &operator<<(
+    std::ostream &os,
+    const Kokkos::Tools::Experimental::DeviceType &deviceType) {
+  switch (deviceType) {
+    case Kokkos::Tools::Experimental::DeviceType::Serial:
+      os << "CPU";
+      output += "Type: CPU";
+      break;
+    case Kokkos::Tools::Experimental::DeviceType::OpenMP:
+      os << "OpenMP";
+      output += "Type: OpenMP";
+      break;
+    case Kokkos::Tools::Experimental::DeviceType::Cuda:
+      os << "cuda";
+      output += "Type: CUDA";
+      break;
+    case Kokkos::Tools::Experimental::DeviceType::HIP:
+      os << "hip";
+      output += "Type: HIP";
+      break;
+    case Kokkos::Tools::Experimental::DeviceType::OpenMPTarget:
+      os << "openmptarget";
+      output += "Type: openmptarget";
+      break;
+    case Kokkos::Tools::Experimental::DeviceType::HPX:
+      os << "hpx";
+      output += "Type: hpx";
+      break;
+    case Kokkos::Tools::Experimental::DeviceType::Threads:
+      os << "threads";
+      output += "Type: threads";
+      break;
+    case Kokkos::Tools::Experimental::DeviceType::SYCL:
+      os << "sycl";
+      output += "Type: SYCL";
+      break;
+    case Kokkos::Tools::Experimental::DeviceType::OpenACC:
+      os << "openacc";
+      output += "Type: OPENACC";
+      break;
+
+    default:
+      os << "Unknown Device Type";
+      output += "Type: Uknown Device Type";
+      break;
  }
-  time_t total_time;
-  time_t end_time;
-  time(&end_time);
-  std::cout << "End Time: " << end_time << "\nStart Time: " << start_time
-            << "\n";
-  total_time = end_time - start_time;
-
-  std::cout << "The kokkos library was alive for " << total_time << " seconds."
-            << std::endl;
+  return os;


Define a function such as

kokkos-tools/profiling/space-time-stack/kp_space_time_stack.cpp

Lines 54 to 67 in 5e895a3

Space get_space(SpaceHandle const& handle) {

// check that name starts with "Cuda"

if (strncmp(handle.name, "Cuda", 4) == 0) return SPACE_CUDA;

// check that name starts with "SYCL"

if (strncmp(handle.name, "SYCL", 4) == 0) return SPACE_SYCL;

// check that name starts with "OpenMPTarget"

if (strncmp(handle.name, "OpenMPTarget", 12) == 0) return SPACE_OMPT;

// check that name starts with "HIP"

if (strncmp(handle.name, "HIP", 3) == 0) return SPACE_HIP;

if (strcmp(handle.name, "Host") == 0) return SPACE_HOST;

abort();

return SPACE_HOST;

}

instead. We don't want to make Kokkos::Tools::Experimental::DeviceType implicitly printable.

twilk10 · 2024-08-06T13:56:34Z

This is an update to the Kokkos-tool variorum connector and can be used to get the power data an
Tested on incremental HIP test on exascale machine using AMD style architecture.
The idea of change is to make this tool usable and change the functionality to retrieve power information at the kernel level as that is a more modern use for the tool.
This is a step in the direction of allowing users to measure their energy consumption while using Kokkos.
Features to be updated
-updated json parsing
-updated what is to be printed out
Features that were updated in variorum but others
-API to get power
Features implemented
-Equation to estimate energy usage until energy API is updated
-Moving the capture of data to the hooks so that the power data being retrieved is from inside the kernel
Outputs include device Id device type device kernel and power alongside power estimation
Also Outputs to a file that is made on the system. User can provide an environment variable to be used by uncommenting code but if not variorum will make a file for the user to put the information in.

Compatible with variorum version 6.0
Sample output;
name: Z4mainEUllRlE_ Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation 2679.104000

Rombur · 2024-08-06T14:01:57Z

You pushed your build directory. You need to remove it.

masterleinad · 2024-08-06T14:30:11Z

profiling/variorum-connector/kp_variorium_connector-temp.cpp

Remove this file.

masterleinad · 2024-08-06T14:30:36Z

profiling/variorum-connector/kp_variorum_connector-temp.cpp

Remove this file.

masterleinad · 2024-08-06T14:31:26Z

profiling/variorum-connector/varconnectsample

Remove this file.

masterleinad · 2024-08-06T14:32:07Z

profiling/variorum-connector/variorum-connector.cpp

@@ -1,4 +1,5 @@
-//@HEADER
+//@HEADER140


masterleinad · 2024-08-06T14:32:27Z

profiling/variorum-connector/variorum-connector.cpp

+//#include <cxxabi.h>
+//#include <dlfcn.h>


Suggested change

//#include <cxxabi.h>

//#include <dlfcn.h>

If you don't need it, remove it.

masterleinad · 2024-08-06T14:45:40Z

profiling/variorum-connector/variorum-connector.cpp

+    if (gdevID == 0) {
+      switch (gdevID) {
+        case 1: {
+          json_t* gpu_value = json_object_get(power_gpu_watts, "GPU_1");


Suggested change

json_t* gpu_value = json_object_get(power_gpu_watts, "GPU_1");

json_t* gpu_value = json_object_get(power_gpu_watts, "GPU_" + std::to_string(gdevID));

and remove all the switch cases.

masterleinad · 2024-08-06T14:47:13Z

profiling/variorum-connector/variorum-connector.cpp

+      output = "  Energy Estimation " + std::to_string(((power1 + power2) / 2) *
+                                                       ((temp - time1) * .001));


What's the unit?

masterleinad · 2024-08-06T14:47:24Z

profiling/variorum-connector/variorum-connector.cpp

    }
  } catch (int e) {
    std::cout << "No MPI Option provided, not using per rank output"
              << std::endl;
-    usingMPI = false;
+    //  usingMPI = false;


Suggested change

// usingMPI = false;

masterleinad · 2024-08-06T14:49:05Z

profiling/variorum-connector/variorum-connector.cpp

+std::ostream& operator<<(
+    std::ostream& os,
+    const Kokkos::Tools::Experimental::ExecutionSpaceIdentifier& identifier) {
+  gdevID = identifier.device_id;
+
+  output +=
+      " Device ID: " + std::to_string(identifier.device_id) +
+      " Instance ID: " + std::to_string(identifier.instance_id) +
+      " DeviceType: " +
+      deviceTypeToString(static_cast<Kokkos::Tools::Experimental::DeviceType>(
+          identifier.device_id));
+
+  writeToFile(filename, output);
+  return os;
 }


Please don't introduce an overload for operator<< but use a stringstream or write to the file immediately instead.

masterleinad · 2024-08-06T14:50:17Z

profiling/variorum-connector/variorum-connector.cpp

+  writeToFile(filename, "name: " + std::string(name));
+  gdevID      = devID;
+  auto result = Kokkos::Tools::Experimental::identifier_from_devid(devID);
+  std::cout << result;


Please don't print to cout anywhere but only to the output file.

masterleinad · 2024-08-06T15:42:13Z

Sample output; name: Z4mainEUllRlE_ Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation 2679.104000

Please provide output from the incremental tests or so.

masterleinad · 2024-08-07T19:57:24Z

profiling/variorum-connector/variorum-connector.cpp

+// The variable below assist with logic and serve as a way to save measurements
+// for calaculation
+int power       = -1;
+int power1      = 0;
+int power2      = 0;
+int filemake    = -1;
+long long time1 = 0;
+long long time2 = 0;


Can you be more precise?

masterleinad · 2024-08-07T19:57:51Z

profiling/variorum-connector/variorum-connector.cpp

+// This is a global variable instituted to track what the device ID is
+uint32_t gdevID = -1;


You shouldn't need a global variable for that.

Thank you for catching that.

masterleinad · 2024-08-07T19:58:40Z

profiling/variorum-connector/variorum-connector.cpp

-bool verbosePrint = false;
-bool mpiOutPut    = false;
+
+bool verbosePrint = true;


Do you still need this? What does it do?

masterleinad · 2024-08-07T19:59:13Z

profiling/variorum-connector/variorum-connector.cpp

+}
+
+void printFile() {
+  // std::string filename = getFile("KOKKOS_TOOLS_VARIORUM_OUTPUT_FILE");


Why is this commented?

To give the option to users to use environment variable if they chose to or have the file variorumoutput.txt put directly into their directory.

Right but why is it not active? If you want to fall back to "variorumoutput.txt", you should not error out in getFile but rather use that if the environment variable couldn't be parsed correctly.

That is a good idea

masterleinad · 2024-08-07T19:59:29Z

profiling/variorum-connector/variorum-connector.cpp

@@ -89,19 +130,19 @@ bool mpiOutPut    = false;
 // value.
 std::string variorum_print_power_call() {
  std::string outputString;
-  json_t* power_obj = json_object();
+  char* s = NULL;


Suggested change

char* s = NULL;

char* s = nullptr;

masterleinad · 2024-08-07T20:00:56Z

profiling/variorum-connector/variorum-connector.cpp

+    json_t* root            = NULL;
+    json_t* socket_0        = NULL;
+    json_t* timestamp_value = NULL;
+    json_t* power_gpu_watts = NULL;
+    json_t* gpu_0_value     = NULL;


Suggested change

json_t* root = NULL;

json_t* socket_0 = NULL;

json_t* timestamp_value = NULL;

json_t* power_gpu_watts = NULL;

json_t* gpu_0_value = NULL;

json_t* root = nullptr;

json_t* socket_0 = nullptr;

json_t* timestamp_value = nullptr;

json_t* power_gpu_watts = nullptr;

json_t* gpu_0_value = nullptr;

masterleinad · 2024-08-07T20:01:55Z

profiling/variorum-connector/variorum-connector.cpp

+      fprintf(stderr, "Expected 'power_gpu_watts' to be an object.\n");
+    }
+
+    std::cout << "Device ID: " << gdevID << std::endl;


Suggested change

std::cout << "Device ID: " << gdevID << std::endl;

profiling/variorum-connector/variorum-connector.cpp

masterleinad · 2024-08-07T20:03:20Z

profiling/variorum-connector/variorum-connector.cpp

  }
 }
-


Please revert deleting this line.

Which line are you referring to.

Look at the comments via https://github.com/kokkos/kokkos-tools/pull/265/files.

masterleinad · 2024-08-07T20:04:16Z

profiling/variorum-connector/variorum-connector.cpp

+          result.device_id));
+  writeToFile(filename, output);
+
+  variorum_call();


Pass the device id here.

Co-authored-by: Daniel Arndt <[email protected]>

masterleinad · 2024-08-08T19:39:24Z

profiling/variorum-connector/variorum-connector.cpp

+int power = -1;  // This variable is used to keep track of if variorum call has
+                 // been called twice to check if the variables need to be reset
+                 // for the next kernel


Wouldn't a bool be better?

masterleinad · 2024-08-08T19:39:52Z

profiling/variorum-connector/variorum-connector.cpp

+int filemake = -1;  // This variable is to check if the file has already been
+                    // made earlier in the program


Wouldn't a bool make more sense?

masterleinad · 2024-08-08T19:40:06Z

profiling/variorum-connector/variorum-connector.cpp

+double power1 =
+    0;  // This variable is used to obtain the initial power for calculation
+double power2 =
+    0;  // This variable is used to obtain the final power for calculation


Could be a double array.

masterleinad · 2024-08-08T19:40:17Z

profiling/variorum-connector/variorum-connector.cpp

+long long time1 =
+    0;  // This variable is used to obtain the initial time for calculation
+long long time2 =
+    0;  // This variable is used to obtain the final time for calculation


Could be a double array.

masterleinad · 2024-08-08T19:41:34Z

profiling/variorum-connector/variorum-connector.cpp

 inline std::string getFile(const char* env_var_name) {
  char* parsed_output_file = getenv(env_var_name);
  if (!parsed_output_file) {
    std::cerr << "Couldn't parse KOKKOS_TOOLS_VARIORUM_OUTPUT_FILE environment "
-                 "variable!\n";
-    std::abort();
+                 "variable! Printed to variorumoutput.txt\n";
+    //  parsed_output_file = "variorumoutput.txt";
+    char vararr[19]    = "variorumoutput.txt";
+    parsed_output_file = vararr;
  }
  return std::string(parsed_output_file);
 }


Wouldn't it be sufficient to call this when Kokkos is initialized?

masterleinad · 2024-08-08T19:43:16Z

profiling/variorum-connector/variorum-connector.cpp

+    //  parsed_output_file = "variorumoutput.txt";
+    char vararr[19]    = "variorumoutput.txt";
+    parsed_output_file = vararr;


Suggested change

// parsed_output_file = "variorumoutput.txt";

char vararr[19] = "variorumoutput.txt";

parsed_output_file = vararr;

return "variorumoutput.txt";

masterleinad · 2024-08-08T19:44:28Z

profiling/variorum-connector/variorum-connector.cpp

@@ -140,7 +149,7 @@ std::string variorum_print_power_call() {
  power_node = json_real_value(json_object_get(power_obj, "power_node"));
  const char* hostnameChar =
      json_string_value(json_object_get(power_obj, "hostname"));
-
+}


This looks suspicious.

masterleinad · 2024-08-08T19:46:17Z

profiling/variorum-connector/variorum-connector.cpp

-    std::cout << "No MPI Option provided, not using per rank output"
-              << std::endl;
-
-  }
  // Simple timer code to keep track of the general amount of time the
  // application ran for.


Is any of this still used?

twilk10 · 2024-08-09T18:06:24Z

Current out put with Incremental test

name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 270.228000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 232.134000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 233.646000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 235.074000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 233.436000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 232.008000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 234.276000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 231.882000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 232.050000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 232.176000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 231.462000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 233.394000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 234.108000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 234.528000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 231.840000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 231.462000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 231.378000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 233.520000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 233.646000
name: Reduction Device ID: 0 Instance ID: 1 DeviceType: SERIAL Energy Estimation in Joules 231.042000

Tyler Wilkerson (kuw) added 3 commits July 15, 2024 14:41

first commit number 3

2fb4b91

variorum updates

554b7f9

variorum updates

c4e7b0a

masterleinad reviewed Jul 19, 2024

View reviewed changes

clang update

4e88454

vlkale changed the title ~~Varconnect~~ Variorium Connector: kernel granularity and JSON Jul 23, 2024

masterleinad reviewed Jul 23, 2024

View reviewed changes

Tyler Wilkerson (kuw) and others added 2 commits August 5, 2024 15:37

updated based on concerns

1abd450

Merge branch 'kokkos:develop' into varconnect

0c5d95f

Tyler Wilkerson (kuw) added 2 commits August 6, 2024 10:15

remove build directory

e9971f8

varconnect deleted

69c8a4c

masterleinad requested changes Aug 6, 2024

View reviewed changes

remove unneeded files

6c8a4c7

tpatki mentioned this pull request Aug 6, 2024

Update Kokkos Variorum port to report energy. LLNL/variorum#574

Open

Tyler Wilkerson (kuw) added 2 commits August 7, 2024 15:44

addressing comments on pull request

1932e24

addressing comments on pull request

090eda4

masterleinad requested changes Aug 7, 2024

View reviewed changes

twilk10 and others added 3 commits August 8, 2024 10:40

Update profiling/variorum-connector/variorum-connector.cpp

803f61f

Co-authored-by: Daniel Arndt <[email protected]>

update based on comments on pull request 10:48 8/8

eebc9f4

updates based off of pull requests 15:37 8/8

4aefd92

masterleinad reviewed Aug 8, 2024

View reviewed changes

updates based off of pull request comments 14:04 8/9

56ac9cd

Tyler Wilkerson (kuw) and others added 2 commits August 9, 2024 14:20

updates based off of pull request comments 14:20 8/9

7e33048

Clean up the variorum connector some more

f4a765d

		// std::cout << "Number of Sockets: " << num_sockets <<
		// std::endl;

	Space get_space(SpaceHandle const& handle) {
	// check that name starts with "Cuda"
	if (strncmp(handle.name, "Cuda", 4) == 0) return SPACE_CUDA;
	// check that name starts with "SYCL"
	if (strncmp(handle.name, "SYCL", 4) == 0) return SPACE_SYCL;
	// check that name starts with "OpenMPTarget"
	if (strncmp(handle.name, "OpenMPTarget", 12) == 0) return SPACE_OMPT;
	// check that name starts with "HIP"
	if (strncmp(handle.name, "HIP", 3) == 0) return SPACE_HIP;
	if (strcmp(handle.name, "Host") == 0) return SPACE_HOST;

	abort();
	return SPACE_HOST;
	}

	json_t* gpu_value = json_object_get(power_gpu_watts, "GPU_1");
	json_t* gpu_value = json_object_get(power_gpu_watts, "GPU_" + std::to_string(gdevID));

		output = " Energy Estimation " + std::to_string(((power1 + power2) / 2) *
		((temp - time1) * .001));

		// This is a global variable instituted to track what the device ID is
		uint32_t gdevID = -1;

		int filemake = -1; // This variable is to check if the file has already been
		// made earlier in the program

Variorium Connector: kernel granularity and JSON #265

Are you sure you want to change the base?

Variorium Connector: kernel granularity and JSON #265

Conversation

twilk10 commented Jul 19, 2024

masterleinad commented Jul 19, 2024

masterleinad 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

masterleinad commented Jul 19, 2024

vlkale commented Jul 19, 2024

twilk10 commented Jul 19, 2024

vlkale commented Jul 20, 2024

twilk10 commented Jul 20, 2024

vlkale commented Jul 23, 2024

vlkale commented Jul 23, 2024

twilk10 commented Jul 23, 2024

twilk10 commented Jul 23, 2024

twilk10 commented Jul 23, 2024

vlkale commented Jul 23, 2024

twilk10 commented Jul 23, 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

twilk10 commented Aug 6, 2024

Rombur commented Aug 6, 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

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

masterleinad commented Aug 6, 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

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

twilk10 commented Aug 9, 2024 • edited Loading

twilk10 commented Aug 9, 2024 •

edited

Loading