C ++ - Code zum Generieren zufälliger DAGs
Ich habe vor einiger Zeit den folgenden C ++ - Code geschrieben, um zufällige Diagramme für ein Projekt zu generieren, an dem ich gearbeitet habe:
#include "stdafx.h"
#include <iostream>
#include <algorithm> // std::min_element, std::max_element
#include <sstream>
#include <fstream>
#include <string>
#include <iterator>
#include <random>
#include <vector>
#define NoOfNodes 30
struct GPU_data
{
int number_Copies;
int workItems;
int workGroups;
bool memory;
double power_consumption;
double execTime;
};
struct DAG_data
{
int processid; //Node's ID
int PEid; //Processor's ID to which node is assigned
std::vector<GPU_data> Node_config;
int precede;
int follow; //nodes following this node
int noOfCopies;
double transData;
double ExecTime;
double powerDraw;
};
void CreateandAssignEdges(DAG_data Sample, int NoOfEdges)
{
unsigned int i = 0;
if (Sample.processid == 0)
{
//parent process- so there will be no edges
Sample.precede = 0;
Sample.follow = rand()% NoOfEdges + 1;
}
else if (Sample.processid == NoOfNodes - 1)
{
//sink process- so there will be no following edges
Sample.follow = 0;
}
else
{
//which nodes will the edges connect to (Anywhere from among the following nodes, including the sink node)
Sample.follow = (Sample.processid + 1) + (std::rand() % (29 - (Sample.processid) + 1));
if (Sample.follow == 30)
{
Sample.follow -= 1;
}
}
}
DAG_data EdgeAssignment(DAG_data Sample, int NoOfEdges)
{
unsigned int i = 0;
if (Sample.processid == 0)
{
//parent process- so there will be no edges
Sample.precede = 0;
Sample.follow = rand() % NoOfEdges + 1;
return Sample;
}
else if (Sample.processid == NoOfNodes - 1)
{
//sink process- so there will be no following edges
Sample.follow = 0;
return Sample;
}
else
{
//which nodes will the edges connect to (Anywhere from among the following nodes, including the sink node)
Sample.follow = (Sample.processid + 1) + (std::rand() % (29 - (Sample.processid) + 1));
return Sample;
}
}
//Sample->precede = rand() % NoOfEdges;
//Sample->follow = rand() % NoOfEdges;
////Preceding and following edges of a node should not be the same.
//while (Sample->precede > Sample->follow || Sample->precede == Sample->follow)
//{
// //assign both edges again
// Sample->follow = rand() % NoOfEdges;
// Sample->precede = rand() % NoOfEdges;
//}
void whenPEisGPU(DAG_data Sample, int processorID)
{
GPU_data emptySet;
int i = 0;
int NoOfConfigs = rand() % 5;
GPU_data* sub_tasks = &emptySet;
while (i != NoOfConfigs)
{
sub_tasks->memory = rand() % 1;
sub_tasks->number_Copies = rand() % 3;
sub_tasks->workGroups = rand() % 10 +1;
sub_tasks->workItems = rand() % (sub_tasks->workGroups * 2) + 1;
sub_tasks->power_consumption = rand() % 250;
sub_tasks->execTime = rand() % (int)(Sample.ExecTime / 2);
Sample.Node_config.push_back(*sub_tasks);
i++;
}
}
void PESpecificParameters(DAG_data Sample, int processorID)
{
if (processorID == 0)
{
Sample.ExecTime = rand() % 100;
Sample.powerDraw = 0.0;
Sample.noOfCopies = 0;
}
else if (processorID == 1)
{
Sample.PEid = processorID;
//whenPEisGPU(Sample, processorID);
int i = 0;
int NoOfConfigs = rand() % 5;
GPU_data sub_tasks;
while (i != NoOfConfigs)
{
sub_tasks.memory = rand() % 1;
sub_tasks.number_Copies = rand() % 3+1;
sub_tasks.workGroups = rand() % 10 + 1;
sub_tasks.workItems = rand() % (sub_tasks.workGroups * 2) + 1;
sub_tasks.power_consumption = rand() % 250;
sub_tasks.execTime = rand() % (int)(Sample.ExecTime / 2);
Sample.Node_config.push_back(sub_tasks);
i++;
}
}
}
DAG_data PEParameters(DAG_data Sample, int processorID)
{
if (processorID == 0)
{
Sample.ExecTime = rand() % 100;
Sample.powerDraw = 0.0;
Sample.noOfCopies = 0;
return Sample;
}
else if (processorID == 1)
{
Sample.PEid = processorID;
//whenPEisGPU(Sample, processorID);
int i = 0;
int NoOfConfigs = rand() % 5;
GPU_data sub_tasks;
while (i != NoOfConfigs)
{
sub_tasks.memory = rand() % 1;
sub_tasks.number_Copies = rand() % 3 + 1;
sub_tasks.workGroups = rand() % 10 + 1;
sub_tasks.workItems = rand() % (sub_tasks.workGroups * 2) + 1;
sub_tasks.power_consumption = rand() % 250;
sub_tasks.execTime = rand() % (int)(Sample.ExecTime / 2) + 1;
Sample.Node_config.push_back(sub_tasks);
i++;
}
return Sample;
}
}
void generateEdges(std::vector<DAG_data> &myTaskGraph)
{
unsigned int i = 0;
while (i != myTaskGraph.size())
{
for (unsigned int j = (myTaskGraph[i].processid)+1; j < myTaskGraph.size(); j++)
{
if (myTaskGraph[i].follow == 30)
{
myTaskGraph[i].follow -= 1;
}
//create an edge between the current node and any of its following nodes according to the following random number
if (rand() % 100 < 30)
{
myTaskGraph[i].follow = j;
break;
}
}
i++;
}
}
int main()
{
DAG_data emptyDAG;
unsigned int i = 0;
std::ofstream myFile;
std::vector<DAG_data> All_DAGs;
while (i != NoOfNodes)
{
DAG_data DAG1;
DAG1.processid = i;
DAG1.transData = i + 1;
DAG1.PEid = 0;
DAG1= PEParameters(DAG1, DAG1.PEid);
DAG1= EdgeAssignment(DAG1, 10);
All_DAGs.push_back(DAG1);
//DAG1.Node_config.clear();
i++;
}
generateEdges(All_DAGs);
for (int h = 0; h < All_DAGs.size(); h++)
{
if (h % 2 != 0)
{
DAG_data forNewPE =PEParameters(All_DAGs[h], 1);
All_DAGs.push_back(forNewPE);
All_DAGs[h].Node_config.clear();
if (All_DAGs[h].processid ==29)
{
break;
}
}
}
myFile.open("TG_Data_30NewEdges.txt");
for (int i = 0; i < All_DAGs.size(); i++)
{
myFile << "Node id: " << All_DAGs[i].processid << std::endl;
myFile << "Following Edge: " << All_DAGs[i].follow << std::endl;
myFile << "Transfer Data: " << All_DAGs[i].transData << std::endl;
myFile << "Node PE: " << All_DAGs[i].PEid << std::endl;
if (All_DAGs[i].PEid == 0)
{
myFile << "Execution time: " << All_DAGs[i].ExecTime << std::endl;
}
else
{
myFile << "-------------------------------" << std::endl;
for (int j = 0; j < All_DAGs[i].Node_config.size(); j++)
{
myFile << "Execution time: " << All_DAGs[i].Node_config[j].execTime << std::endl;
myFile << "Copies: " << All_DAGs[i].Node_config[j].number_Copies << std::endl;
myFile << "Memory: " << All_DAGs[i].Node_config[j].memory << std::endl;
myFile << "Work-Items: " << All_DAGs[i].Node_config[j].workItems << std::endl;
myFile << "Work-Groups: " << All_DAGs[i].Node_config[j].workGroups << std::endl;
myFile << "Power: " << All_DAGs[i].Node_config[j].power_consumption << std::endl;
myFile << "++++++++++++++++++" << std::endl;
}
}
myFile << "=================" << std::endl;
}
myFile.close();
std::cout << "DONE NOW." << std::endl;
std::cin.get();
}
Der Code hat sein Ziel für mich erfüllt, aber es gibt viel Raum für Verbesserungen für diesen Code. Bitte geben Sie an, wie dieser Code neu geschrieben werden kann, um die gewünschten C ++ - Praktiken besser einzuhalten.
Antworten
Wichtige Fehler:
Ihr Zufall ist nicht zufällig (Startwert)
Ihr Zufall ist nicht einheitlich (verwenden Sie gleichmäßige Verteilungen, anstatt nur den Modul zu verwenden, der die Verteilung verzerrt).
precedeist oft nicht initialisiert;NoOfConfigswird oft nicht initialisiert und nie benutzt?Die letzte Schleife vor dem Schreiben der Ausgabedatei ändert die Sammlung während der Iteration :
for (size_t h = 0; h < nodes.size(); h++) { // ... nodes.push_back(forNewPE);Dies ist ein Anti-Muster. Sie kommen nur gerade damit durch
if (nodes[h].processid == 29) { break; }was natürlich unter magischen Zahlen leidet und stattdessen leicht in die Schleifenbedingung versetzt werden könnte:
for (size_t h = 0; h < NoOfNodes; ++h) {void PESpecificParameters(DAG_data Sample, int processorID)ist nicht benutzt.Bei Verwendung hat es niemals Auswirkungen (da es netiher Rückgabewerte hat und keine Verweise auf etwas außerhalb enthält).
Das gleiche mit
whenPEisGPUNach dem Entfernen von doppeltem Code sieht es so aus, als wäre
PEParameterses identisch mitPESpecificParameters(siehe unten)Ebenso
CreateandAssignEdgeswar unbenutzt und scheint zu duplizierenEdgeAssignment?
Wichtige Hinweise:
Benennung!
DAG_Databedeutet so gut wie nichts. Ihr Diagrammmodell repräsentiert etwas im wirklichen Leben. Die Tatsache, dass es sich um eine DAG handelt, entspricht dem Aufrufen von Variablen "textstring" anstelle von "FirstName" und "ZipCode".Extrahieren Sie einige Funktionen. Verwenden Sie sie, um
- getrennte Verantwortlichkeiten,
- Abstraktionsebenen
- Reduzieren Sie Doppelarbeit
Gruppieren Sie optional verwandte Funktionen mit ihren Daten in Klassen (siehe Abschnitt "BONUS" unten).
Hier kommt ein Schlag für Schlag von Dingen, die ich angesprochen habe:
Verwenden Sie Warnungen (-Wall -Wextra -pedantic mindestens) und schlagen Sie sie:
test.cpp:43:18: warning: unused variable ‘i’ [-Wunused-variable] 43 | unsigned int i = 0; test.cpp:74:18: warning: unused variable ‘i’ [-Wunused-variable] 74 | unsigned int i = 0; test.cpp:119:39: warning: unused parameter ‘processorID’ [-Wunused-parameter] 119 | void whenPEisGPU(DAG_data Sample, int processorID) test.cpp:259:23: warning: comparison of integer expressions of different signedness: ‘int’ and ‘std::vector<DAG_data>::size_type’ {aka ‘long unsigned int’} [-Wsign-compare] 259 | for (int h = 0; h < All_DAGs.size(); h++) test.cpp:277:23: warning: comparison of integer expressions of different signedness: ‘int’ and ‘std::vector<DAG_data>::size_type’ {aka ‘long unsigned int’} [-Wsign-compare] 277 | for (int i = 0; i < All_DAGs.size(); i++) test.cpp:290:31: warning: comparison of integer expressions of different signedness: ‘int’ and ‘std::vector<GPU_data>::size_type’ {aka ‘long unsigned int’} [-Wsign-compare] 290 | for (int j = 0; j < All_DAGs[i].Node_config.size(); j++) test.cpp:204:1: warning: control reaches end of non-void function [-Wreturn-type] 204 | }Änderungen:
CreateandAssignEdges: - unsigned int i = 0; EdgeAssignment: - unsigned int i = 0; -void whenPEisGPU(DAG_data Sample, int processorID) +void whenPEisGPU(DAG_data Sample, int /*processorID*/) PEParameters: + throw std::range_error("processorID"); - for (int h = 0; h < All_DAGs.size(); h++) + for (size_t h = 0; h < All_DAGs.size(); h++) - for (int i = 0; i < All_DAGs.size(); i++) + for (size_t i = 0; i < All_DAGs.size(); i++) - for (int j = 0; j < All_DAGs[i].Node_config.size(); j++) + for (size_t j = 0; j < All_DAGs[i].Node_config.size(); j++)Das Ausführen der Modernisierungs- / Lesbarkeitsprüfung zeigt viele magische Zahlenwarnungen und einige einfache Verbesserungen:
clang-apply-replacements version 9.0.0 clang-tidy-9 -header-filter=.* -checks=-*,readability-*,modernize-*,-modernize-use-trailing-return-type -export-fixes /tmp/tmp6CfbSr/tmpYGk6CX.yaml -p=/home/sehe/Projects/stackoverflow /home/sehe/Projects/stackoverflow/test.cpp /home/sehe/Projects/stackoverflow/test.cpp:59:66: warning: 29 is a magic number; consider replacing it with a named constant [readability-magic-numbers] Sample.follow = (Sample.processid + 1) + (std::rand() % (29 - (Sample.processid) + 1)); ^ /home/sehe/Projects/stackoverflow/test.cpp:61:30: warning: 30 is a magic number; consider replacing it with a named constant [readability-magic-numbers] if (Sample.follow == 30) ^ /home/sehe/Projects/stackoverflow/test.cpp:81:5: warning: do not use 'else' after 'return' [readability-else-after-return] else if (Sample.processid == NoOfNodes - 1) ^~~~~ /home/sehe/Projects/stackoverflow/test.cpp:92:66: warning: 29 is a magic number; consider replacing it with a named constant [readability-magic-numbers] Sample.follow = (Sample.processid + 1) + (std::rand() % (29 - (Sample.processid) + 1)); ^ /home/sehe/Projects/stackoverflow/test.cpp:119:32: warning: 5 is a magic number; consider replacing it with a named constant [readability-magic-numbers] int NoOfConfigs = rand() % 5; ^ /home/sehe/Projects/stackoverflow/test.cpp:123:29: warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion] sub_tasks->memory = rand() % 1; ^ (( ) != 0) /home/sehe/Projects/stackoverflow/test.cpp:125:42: warning: 10 is a magic number; consider replacing it with a named constant [readability-magic-numbers] sub_tasks->workGroups = rand() % 10 +1; ^ /home/sehe/Projects/stackoverflow/test.cpp:127:49: warning: 250 is a magic number; consider replacing it with a named constant [readability-magic-numbers] sub_tasks->power_consumption = rand() % 250; ^ /home/sehe/Projects/stackoverflow/test.cpp:138:36: warning: 100 is a magic number; consider replacing it with a named constant [readability-magic-numbers] Sample.ExecTime = rand() % 100; ^ /home/sehe/Projects/stackoverflow/test.cpp:148:36: warning: 5 is a magic number; consider replacing it with a named constant [readability-magic-numbers] int NoOfConfigs = rand() % 5; ^ /home/sehe/Projects/stackoverflow/test.cpp:152:32: warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion] sub_tasks.memory = rand() % 1; ^ (( ) != 0) /home/sehe/Projects/stackoverflow/test.cpp:154:45: warning: 10 is a magic number; consider replacing it with a named constant [readability-magic-numbers] sub_tasks.workGroups = rand() % 10 + 1; ^ /home/sehe/Projects/stackoverflow/test.cpp:156:52: warning: 250 is a magic number; consider replacing it with a named constant [readability-magic-numbers] sub_tasks.power_consumption = rand() % 250; ^ /home/sehe/Projects/stackoverflow/test.cpp:170:36: warning: 100 is a magic number; consider replacing it with a named constant [readability-magic-numbers] Sample.ExecTime = rand() % 100; ^ /home/sehe/Projects/stackoverflow/test.cpp:177:5: warning: do not use 'else' after 'return' [readability-else-after-return] else if (processorID == 1) ^~~~~ /home/sehe/Projects/stackoverflow/test.cpp:182:36: warning: 5 is a magic number; consider replacing it with a named constant [readability-magic-numbers] int NoOfConfigs = rand() % 5; ^ /home/sehe/Projects/stackoverflow/test.cpp:186:32: warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion] sub_tasks.memory = rand() % 1; ^ (( ) != 0) /home/sehe/Projects/stackoverflow/test.cpp:188:45: warning: 10 is a magic number; consider replacing it with a named constant [readability-magic-numbers] sub_tasks.workGroups = rand() % 10 + 1; ^ /home/sehe/Projects/stackoverflow/test.cpp:190:52: warning: 250 is a magic number; consider replacing it with a named constant [readability-magic-numbers] sub_tasks.power_consumption = rand() % 250; ^ /home/sehe/Projects/stackoverflow/test.cpp:211:42: warning: 30 is a magic number; consider replacing it with a named constant [readability-magic-numbers] if (myTaskGraph[i].follow == 30) ^ /home/sehe/Projects/stackoverflow/test.cpp:216:26: warning: 100 is a magic number; consider replacing it with a named constant [readability-magic-numbers] if (rand() % 100 < 30) ^ /home/sehe/Projects/stackoverflow/test.cpp:216:32: warning: 30 is a magic number; consider replacing it with a named constant [readability-magic-numbers] if (rand() % 100 < 30) ^ /home/sehe/Projects/stackoverflow/test.cpp:246:36: warning: 10 is a magic number; consider replacing it with a named constant [readability-magic-numbers] DAG1= EdgeAssignment(DAG1, 10); ^ /home/sehe/Projects/stackoverflow/test.cpp:264:41: warning: 29 is a magic number; consider replacing it with a named constant [readability-magic-numbers] if (All_DAGs[h].processid ==29) ^ /home/sehe/Projects/stackoverflow/test.cpp:274:5: warning: use range-based for loop instead [modernize-loop-convert] for (size_t i = 0; i < All_DAGs.size(); i++) ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ (auto & All_DAG : All_DAGs) 7510 warnings generated. Suppressed 7485 warnings (7485 in non-user code). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. Applying fixes ...Nehmen Sie zumindest sofort die Fernkampfschleife:
for (auto& DAG : All_DAGs) { myFile << "Node id: " << DAG.processid << std::endl; myFile << "Following Edge: " << DAG.follow << std::endl; myFile << "Transfer Data: " << DAG.transData << std::endl; myFile << "Node PE: " << DAG.PEid << std::endl; if (DAG.PEid == 0) { myFile << "Execution time: " << DAG.ExecTime << std::endl; } else { myFile << "-------------------------------" << std::endl; for (auto& cfg : DAG.Node_config) { myFile << "Execution time: " << cfg.execTime << std::endl; myFile << "Copies: " << cfg.number_Copies << std::endl; myFile << "Memory: " << cfg.memory << std::endl; myFile << "Work-Items: " << cfg.workItems << std::endl; myFile << "Work-Groups: " << cfg.workGroups << std::endl; myFile << "Power: " << cfg.power_consumption << std::endl; myFile << "++++++++++++++++++" << std::endl; } } myFile << "=================" << std::endl; }Trennen Sie die Initialisierung nicht unnötig von der Deklaration.
std::ofstream myFile; // 40 lines... myFile.open("TG_Data_30NewEdges.txt");Ressourcenverwaltung nicht unnötig manuell:
myFile.close();Das RAII-Muster von C ++ bedeutet, dass die Datei immer geschlossen wird.
{ std::ofstream output("TG_Data_30NewEdges.txt"); for (auto& DAG : All_DAGs) { // ... } }Hinweis: Ich habe auch in
myFileetwas Beschreibenderes umbenannt.Zeit, einige Funktionen für die oben genannten zu extrahieren:
std::ofstream output("TG_Data_30NewEdges.txt"); writeReport(output, All_DAGs);Und dann woanders:
using DAGs = std::vector<DAG_data>; void writeReport(std::ostream& output, DAGs const& graphs) { for (auto& g : graphs) { // ... } }Schleifen entmystifizieren
unsigned int i = 0; while (i != myTaskGraph.size()) { // ... i++; }Ist konventionell geschrieben als
for (size_t i = 0; i < myTaskGraph.size(); ++i) { // ... }Oder seit c ++ 0x:
for (Node& node : myTaskGraph) { // ... }Ebenso sollten die Schleifen, die Container erstellen, wahrscheinlich eher wie folgt lauten:
Nodes nodes(NoOfNodes); size_t i = 0; for (auto& current : nodes) { current.processid = i; current.transData = i + 1; current.PEid = 0; i++; current = PEParameters(current, current.PEid); current = EdgeAssignment(current, 10); }Und
void whenPEisGPU(Node& node, int /*processorID*/) { int NoOfConfigs = rand() % 5; node.Node_config.assign(NoOfConfigs, {}); for (auto& sub_task : node.Node_config) { sub_task.memory = ((rand() % 1) != 0); sub_task.number_Copies = rand() % 3; sub_task.workGroups = rand() % 10 +1; sub_task.workItems = rand() % (sub_task.workGroups * 2) + 1; sub_task.power_consumption = rand() % 250; sub_task.execTime = rand() % (int)(node.ExecTime / 2); } }usw.
Ich würde sie wahrscheinlich als
std::generate_nAnrufe im wirklichen Leben schreiben , aber vielleicht werden wir natürlich später dort ankommenBenennung. Irgendwo auf halber Strecke des Codes bekommen wir plötzlich einen Blick darauf, womit wir es wirklich zu tun haben:
void generateEdges(std::vector<DAG_data> &myTaskGraph)Also, ich denke wir könnten benennen
DAG_dataNodeoderTask(oder sogarTaskNode?).Ebenso erhalten wir hier subtile Hinweise:
if (Sample.processid == 0) { //parent process- so there will be no edgesund
else if (node.processid == NoOfNodes - 1) { // sink process- so there will be no following edgesRandnotiz: Sie verwenden,
parentals ob es "keine Kanten" bedeutet. Welches ist demonstrativ ungenau, weil Sie sofort tun ein Anhänger Flanke gesetzt. Was Sie zu meinen scheinen, ist das "Elternteil ohne Elternteil", das in einer DAG normalerweise als "Wurzel" bezeichnet wird. Beachten Sie auch, dass Sie eine DAG mit nur 1 Wurzel als Baum bezeichnen können.// Datei unter: Benennung ist wichtig
Also sollten wir das lesbarer machen:
using ProcessID = int; static constexpr size_t NoOfNodes = 30; static constexpr ProcessID RootNodeId = 0; static constexpr ProcessID SinkNodeId = NoOfNodes - 1; // ... static bool constexpr IsSink(ProcessID id) { return SinkNodeId == id; } static bool constexpr IsSink(Node const& node) { return IsSink(node.processid); } // etc?Vielleicht ist es sogar besser, das Ganze zu kombinieren:
enum ProcessID : int { RootNodeId = 0, NoOfNodes = 30, SinkNodeId = NoOfNodes -1, };Dies führt zu einer starken Reduzierung aller magischen Zahlen (
= 0wird= RootNodeIdusw.).Es zwingt uns jedoch, das Problem mit den anderen "magischen" Aufgaben anzugehen:
node.follow = rand() % NoOfEdges + 1; node.follow = (node.processid + 1) + (std::rand() % (29 - (node.processid) + 1));Ich meine, wir wollten diese Probleme ansprechen (weil, ugh und verzerrt zufällig).
Also, lasst uns zufällig ansprechen! Du hast richtig angefangen:
#include <random>aber nie etwas aus dieser Schatzkammer benutzt !
std::mt19937 prng { std::random_device{} () };Jetzt haben wir unseren UniformRandomBitGenerator und haben ihn sicher ausgesät!
Lassen Sie uns einige Hilfsfunktionen erstellen, mit denen wir gleichmäßig verteilte Zahlen generieren können:
Generieren Sie Zahlen bis einschließlich max:
auto gen_number(int max, bool includeZero = true) { using Dist = std::uniform_int_distribution<>; using Param = Dist::param_type; static Dist dist; auto min = includeZero? 0:1; assert(max >= min); return dist(prng, Param(min, max)); }Hinzufügen einer kurzen Hand für [1, max] Zufallsstichprobe:
auto gen_positive(int max) { return gen_number(max, false); }Um ProcessID zu generieren, benötigen wir einige Konvertierungen und können einige Standardeinstellungen für die Bereichsgrenzen annehmen:
ProcessID gen_follower(int from = FirstFollow, int to = NoOfNodes) { using T = std::underlying_type_t<ProcessID>; using Dist = std::uniform_int_distribution<T>; using Param = Dist::param_type; static Param full{static_cast<T>(FirstFollow), static_cast<T>(NoOfNodes)}; static Dist dist(full); return static_cast<ProcessID>(dist(prng, Param(from, to))); }Jetzt können wir die Ausdrücke umformulieren:
// node.follow = rand() % NoOfEdges + 1; node.follow = gen_follower(FirstFollow, NoOfEdges);Und
// node.follow = // (node.processid + 1) + (std::rand() % (29 - (node.processid) + 1)); node.follow = gen_follower(node.processid+1);Viel einfacher, typsicher und einheitlich!
Nun, da gibt es einige seltsame Dinge.
Überall
followwird impliziert, dass es aus derProcessIdDomäne stammt. Der Ausdruckgen_follower(FirstFollow, NoOfEdges)verwendet jedochNoOfEdgesanstelle vonNoOfNodes?!NoOfEdgesist auch nur10für den einen Anruf an fest codiertEdgeAssignment.Sind Sie sicher, dass Sie Follower-Knoten für den Root-Knoten "willkürlich"
[1..10]unabhängig davon einschränken wolltenNoOfNodes?Da nachfolgende Follower immer "Downstream" genommen werden, kann ich davon ausgehen, dass Sie aus einer "ersten 10" -Partition auswählen wollten, um die Wahrscheinlichkeit zu erhöhen, dass Unteraufgaben "Enkelkinder" zeugen. Wenn ja, ist der Name
NoOfEdgesvöllig irreführend und könnte so etwas wieFirstGenerationNodes?)Es gibt zwei Stellen, an denen das Ergebnis dieser Ausdrücke korrigiert wird:
if (myTaskGraph[i].follow == 30) { myTaskGraph[i].follow -= 1; } if (Sample.follow == 30) { Sample.follow -= 1; }Wenn dies der gewünschte Bereich ist, korrigieren Sie einfach Ihre Ausdrücke!
Wie geschrieben, macht es den Code schwer verständlich, verteilt die Verantwortung auf Funktionen (was zu Fehlern führt) und verzerrt die Verteilung weiter:
29ist jetzt ein viel wahrscheinlicheres Edge-Ziel.Ich habe mich entschieden, den Ausdruck so zu korrigieren, dass er der impliziten Absicht dieses anderen Kommentars entspricht:
// which nodes will the edges connect to (Anywhere from among the // following nodes, including the sink node) node.follow = gen_follower(node.processid+1, SinkNodeId);
Codeduplizierung. Die Generierung von Unteraufgaben (
node.Node_config) wird dupliziert, mit einigen falschen Unterschieden, die Fehler sein könnten, aber beabsichtigt sein könnten.Z.B:
sub_task.number_Copies = rand() % 3 + 1;Eine von drei Kopien wird weggelassen,
+1was wahrscheinlich ein Fehler ist.In ähnlicher Weise sehen wir eine Kopie von
sub_task.execTime = rand() % static_cast<int>(node.ExecTime / 2);das fügt ein
+1. Wahrscheinlich vermeidet dies NullexecTimeund ist ein Codegeruch, dass auch dies eine stark typisierte, gleichmäßige reale Zufallsverteilung sein sollte.Es ist schwer zu erraten, was Sie eigentlich
execTimemeinen wollen . Wenn Sie damit meinen, dass die execTime des übergeordneten Knotens immer die Summe seiner Unteraufgaben ergibt, ist dies mit einer Geschäftslogik viel einfacher auszudrücken, als dass die Daten in Ihrer Datenstruktur redundant sind und undokumentierte Invarianten hinzugefügt werden (die wiederum Fehler einladen ).Zum Spaß habe ich hinzugefügt, wie ich die Distribution aus einer Laune heraus schreiben würde:
void distributeExecTime(Node& node) { std::vector<double> weights; std::uniform_real_distribution<> dist; std::generate_n( back_inserter(weights), node.Node_config.size(), [&dist] { return dist(prng); }); auto total_w = std::accumulate(begin(weights), end(weights), 0.); for (size_t i = 0; i < weights.size(); ++i) { node.Node_config[i].execTime = (weights[i]/total_w) * node.ExecTime; } }Für die totale Leistungsaufnahme scheint es ähnliche Dinge zu geben. Vielleicht können Sie powerDraw durch eine Funktion ersetzen:
double powerDraw() const { return std::accumulate(begin(Node_config), end(Node_config), 0.); };
BONUS
Wenn wir über den Rand gehen, können wir uns eine Welt vorstellen, in der die Erzeugung "automatisch" erfolgt, ebenso wie die Berichterstattung:
Ziehen Sie in Betracht, die Generierung in Konstruktoren zu verschieben:
struct GPU_data { int number_Copies = gen_positive(3); int workGroups = gen_positive(10); // order is important! int workItems = gen_positive(workGroups * 2); bool memory = odds(50); double power_consumption = gen_real(249); double execTime = 0; // see distributeExecTime };Hinweis
- Wir verwenden C ++ 11 NSMI, um den Standardkonstruktor für uns zu generieren
struct Node { enum Type { CPUNode, GPUNode }; Type PEid; // Processor's ID to which node is assigned ProcessID processid; // Node's ID Configs sub_tasks; ProcessID follow = RootNodeId; // nodes following this node double transData = 0; double ExecTime = 0; explicit Node(int id, int NoOfEdges = 10) : PEid(CPUNode), processid(ProcessID(id)), transData(id + 1) { PEParameters(); EdgeAssignment(NoOfEdges); } explicit Node(Node const& node) : PEid(GPUNode), processid(node.processid), sub_tasks(), follow(node.follow), transData(node.transData), ExecTime(node.ExecTime) { PEParameters(); } double powerDraw() const; bool isGPU() const { return PEid == GPUNode; } private: void PEParameters(); void EdgeAssignment(int NoOfEdges); void distributeExecTime(); };Jetzt
Nodekann mit seinen manipulierenden Funktionen gruppieren:Bei dieser Art wird davon ausgegangen, dass die Typen an anderer Stelle noch nicht verwendet werden. Falls dies nicht der Fall ist, können wir die Typen unterklassifizieren und vom Objekt-Slicing profitieren, um es wieder in seine Basisklasse zu konvertieren.
Beachten Sie auch, dass mehrere Stellen im Code (PEParameters, output und EdgeAssignment) das Verhalten von PEid ändern, das anscheinend nur zwei gültige Werte hat. Ich habe das geändert, um eine Aufzählung zu erhalten, die diese Tatsache widerspiegelt:
enum Type { CPUNode, GPUNode }; Type PEid; // Processor's ID to which node is assignedAls Übung für den Leser könnte es sinnvoll sein,
Nodezu einem polymorphen Typ zu wechseln , anstatt ständig zu wechseln :using Node = std::variant<CPUNode, GPUNode>;Oder mit virtuellen Typen (Vererbung).
Demo Listing (s)
Alle Revisionen sind hier in einem Kern: https://gist.github.com/sehe/32c07118031a049042bd9fb469355caf/revisions
Lebe auf Coliru
#include <iostream>
#include <algorithm> // std::min_element, std::max_element
#include <fstream>
#include <string>
#include <random>
#include <vector>
#include <cassert>
namespace {
static std::mt19937 prng { std::random_device{} () };
enum ProcessID : int {
RootNodeId /*= 0 */,
NoOfNodes = 30,
FirstFollow = RootNodeId +1,
SinkNodeId = NoOfNodes -1,
};
auto gen_number(int max, bool includeZero = true) {
using Dist = std::uniform_int_distribution<>;
using Param = Dist::param_type;
static Dist dist;
auto min = includeZero? 0:1;
assert(max >= min);
return dist(prng, Param(min, max));
}
auto gen_positive(int max) {
return gen_number(max, false);
}
ProcessID gen_follower(int from = FirstFollow, int to = NoOfNodes) {
using T = std::underlying_type_t<ProcessID>;
using Dist = std::uniform_int_distribution<T>;
using Param = Dist::param_type;
static Param full{static_cast<T>(FirstFollow), static_cast<T>(NoOfNodes)};
static Dist dist(full);
return static_cast<ProcessID>(dist(prng, Param(from, to)));
}
bool odds(int percentage) {
if (percentage == 100)
return true;
assert(percentage > 0 && percentage < 100);
return std::discrete_distribution<bool>(percentage, 100-percentage)(prng);
}
double gen_real(double mean = 100.0, double stddev = 0) {
if (stddev == 0)
stddev = mean/4;
assert(stddev>0);
return std::normal_distribution(mean, stddev)(prng);
}
}
struct GPU_data {
int number_Copies = gen_positive(3);
int workGroups = gen_positive(10); // order is important!
int workItems = gen_positive(workGroups * 2);
bool memory = odds(50);
double power_consumption = gen_real(249);
double execTime = 0; // see distributeExecTime
};
using Configs = std::vector<GPU_data>;
struct Node {
enum Type { CPUNode, GPUNode };
Type PEid; // Processor's ID to which node is assigned
ProcessID processid; // Node's ID
Configs sub_tasks;
ProcessID follow = RootNodeId; // nodes following this node
double transData = 0;
double ExecTime = 0;
explicit Node(int id, int NoOfEdges = 10)
: PEid(CPUNode),
processid(ProcessID(id)),
transData(id + 1)
{
PEParameters();
EdgeAssignment(NoOfEdges);
}
explicit Node(Node const& node)
: PEid(GPUNode),
processid(node.processid),
sub_tasks(),
follow(node.follow),
transData(node.transData),
ExecTime(node.ExecTime)
{
PEParameters();
}
double powerDraw() const {
double total = 0;
for (auto& sub: sub_tasks) {
total += sub.power_consumption;
}
return total;
};
bool isGPU() const { return PEid == GPUNode; }
private:
void PEParameters() {
switch(PEid) {
case CPUNode:
ExecTime = gen_real(100.0);
break;
case GPUNode:
sub_tasks.resize(gen_number(5));
distributeExecTime();
break;
default:
throw std::range_error("PEid");
}
}
void EdgeAssignment(int NoOfEdges) {
if (processid == RootNodeId) {
// parent process- so there will be no edges
follow = gen_follower(FirstFollow, NoOfEdges);
}
else if (processid == SinkNodeId) {
// sink process- so there will be no following edges
follow = RootNodeId;
}
else {
// which nodes will the edges connect to (Anywhere from among the
// following nodes, including the sink node)
follow = gen_follower(processid+1, SinkNodeId);
}
}
void distributeExecTime() {
std::vector<double> weights;
std::uniform_real_distribution<> dist;
std::generate_n(
back_inserter(weights),
sub_tasks.size(),
[&dist] { return dist(prng); });
auto total_w = std::accumulate(begin(weights), end(weights), 0.);
for (size_t i = 0; i < weights.size(); ++i) {
sub_tasks[i].execTime = (weights[i]/total_w) * ExecTime;
}
}
};
using Nodes = std::vector<Node>;
void generateEdges(Nodes& nodes) {
for (Node& node : nodes) {
// Create an edges to following nodes given 30% odds
for (size_t j = node.processid+1; j < nodes.size(); j++) {
if (odds(30)) {
node.follow = static_cast<ProcessID>(j);
break;
}
}
}
}
static std::ostream& operator<<(std::ostream& os, Node const& n);
int main() {
Nodes nodes;
for (auto id = 0; id < NoOfNodes; ++id) {
nodes.emplace_back(id);
}
generateEdges(nodes);
for (size_t h = 0; h < NoOfNodes; h++) {
if (h % 2 == 0)
continue;
nodes.emplace_back(nodes[h]);
nodes[h].sub_tasks.clear();
}
std::ofstream output("TG_Data_30NewEdges.txt");
for (auto& n : nodes) {
output << n << "=================\n";
}
std::cout << "DONE" << std::endl;
}
static std::ostream& operator<<(std::ostream& os, GPU_data const& cfg) {
return os
<< "Execution time: " << cfg.execTime << "\n"
<< "Copies: " << cfg.number_Copies << "\n"
<< "Memory: " << cfg.memory << "\n"
<< "Work-Items: " << cfg.workItems << "\n"
<< "Work-Groups: " << cfg.workGroups << "\n"
<< "Power: " << cfg.power_consumption << "\n";
}
static std::ostream& operator<<(std::ostream& os, Node const& n) {
os << "Node id: " << n.processid << "\n"
<< "Following Edge: " << n.follow << "\n"
<< "Transfer Data: " << n.transData << "\n"
<< "Node powerDraw: " << n.powerDraw() << "\n"
<< "Node PE: " << n.PEid << "\n";
if (n.isGPU()) {
os << "-------------------------------\n";
for (auto& cfg : n.sub_tasks) {
os << cfg << "++++++++++++++++++\n";
}
} else {
os << "Execution time: " << n.ExecTime << "\n";
}
return os;
}
Drucke, z
DONE
Und generiert TG_Data_30NewEdges.txt:
Node id: 0
Following Edge: 1
Transfer Data: 1
Node powerDraw: 1020.61
Node PE: 1
-------------------------------
Execution time: 12.2428
Copies: 1
Memory: 1
Work-Items: 10
Work-Groups: 9
Power: 229.989
++++++++++++++++++
Execution time: 39.2756
Copies: 1
// ...
// 825 lines snipped
// ...
Copies: 3
Memory: 1
Work-Items: 3
Work-Groups: 9
Power: 235.512
++++++++++++++++++
=================
#define NoOfNodes 30
Ich denke, es wäre besser, static constexprhier ein Makro als ein Präprozessor-Makro zu verwenden.
//which nodes will the edges connect to (Anywhere from among the following nodes, including the sink node) Sample.follow = (Sample.processid + 1) + (std::rand() % (29 - (Sample.processid) + 1)); if (Sample.follow == 30) { Sample.follow -= 1; }
Woher kommen die Konstanten 29und woher 30? Sollten sie stattdessen abgeleitet NoOfNodeswerden?
Es ist möglicherweise besser, die C ++ - <random>Bibliothek zu verwenden als std::rand().
CreateandAssignEdges()und EdgeAssignment()sind sehr ähnlich - ich denke, die Duplizierung kann stark reduziert werden.
//Sample->precede = rand() % NoOfEdges; //Sample->follow = rand() % NoOfEdges; ////Preceding and following edges of a node should not be the same. //while (Sample->precede > Sample->follow || Sample->precede == Sample->follow) //{ // //assign both edges again // Sample->follow = rand() % NoOfEdges; // Sample->precede = rand() % NoOfEdges; //}
Solche auskommentierten Codestücke werden häufig zu einem Problem, das veraltet und inkonsistent wird, wenn sich der umgebende Code ändert. Entfernen Sie es entweder oder finden Sie einen Weg, um sicherzustellen, dass es mit dem Rest des Codes kompiliert und Unit-getestet wird.
myFile << "Node id: " << All_DAGs[i].processid << std::endl; myFile << "Following Edge: " << All_DAGs[i].follow << std::endl; myFile << "Transfer Data: " << All_DAGs[i].transData << std::endl; myFile << "Node PE: " << All_DAGs[i].PEid << std::endl;
Es gibt keine wirkliche Notwendigkeit zu spülen myFilejede Aussage - es vorziehen , '\n'zu std::endlfür alle diese (und die meisten / alle übrigen Anwendungen).