Conversation
Ectras
reviewed
Jul 24, 2024
| #define MILQ_BINPACKING_HPP | ||
|
|
||
| #include <Schedule.hpp> | ||
| #include <string> |
There was a problem hiding this comment.
unused include
Suggested change
| #include <string> |
|
|
||
| #include <QuantumResourceManager.hpp> | ||
| #include <qdmi.h> | ||
| #include <vector> |
There was a problem hiding this comment.
unused include
Suggested change
| #include <vector> |
| @@ -0,0 +1,18 @@ | |||
| #ifndef MILQ_BINPACKING_HPP | |||
There was a problem hiding this comment.
I'd prefer #pragma once (this is e.g. also used in the passes repository)
|
|
||
| namespace milq | ||
| { | ||
| std::vector<JobProxy> convert_to_proxies(const std::vector<QuantumTask> &tasks); |
| #ifndef MILQ_BINPACKING_HPP | ||
| #define MILQ_BINPACKING_HPP | ||
|
|
||
| #include <Schedule.hpp> |
There was a problem hiding this comment.
Usually it's quotes for non-system headers:
#include "File.hpp" // user-written file include
#include <vector> // system header include|
|
||
| std::vector<Schedule> improve_population(std::vector<Schedule> &population) | ||
| { | ||
| auto population_copy = population; |
| } | ||
| if (!candidates.empty()) | ||
| { | ||
| std::uniform_int_distribution<int> candidate_distribution( |
There was a problem hiding this comment.
The end of the distribution is inclusive, so this can be out of bounds.
Comment on lines
+176
to
+209
| for (auto &job : bucket->job_proxies) | ||
| { | ||
| auto &machine = *std::min_element( | ||
| schedule.get_machines().begin(), schedule.get_machines().end(), | ||
| [](const Machine *m1, const Machine *m2) | ||
| { return m1->capacity < m2->capacity; }); | ||
| if (machine->buckets.empty()) | ||
| { | ||
| // TODO proprose new cuts ore put them in multipile | ||
| // buckets if necessary | ||
| machine->buckets.push_back(bucket); | ||
| } | ||
| else | ||
| { | ||
| auto &smallest_bucket = *std::min_element( | ||
| machine->buckets.begin(), machine->buckets.end(), | ||
| [&machine](const Bucket *b1, const Bucket *b2) | ||
| { | ||
| return remaining_capacity(b1, machine->capacity) < | ||
| remaining_capacity(b2, machine->capacity); | ||
| }); | ||
| if (remaining_capacity(smallest_bucket, machine->capacity) < | ||
| job->num_qubits) | ||
| { | ||
| // TODO proprose new cuts ore put them in multipile | ||
| // buckets if necessary | ||
| machine->buckets.push_back(bucket); | ||
| } | ||
| else | ||
| { | ||
| smallest_bucket->job_proxies.push_back(job); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not sure if this works. You iterate over the jobs, but then you are adding the whole bucket without breaking. I can't verify easily that this doesn't add jobs twice.
Comment on lines
+254
to
+261
| return std::accumulate( | ||
| population.begin(), population.end(), 0, | ||
| [&x](int sum, Schedule &other) | ||
| { return sum + x.hamming_distance(other); }) < | ||
| std::accumulate( | ||
| population.begin(), population.end(), 0, | ||
| [&y](int sum, Schedule &other) | ||
| { return sum + y.hamming_distance(other); }); |
There was a problem hiding this comment.
The accumulates should go into separate lines for separability
Comment on lines
+305
to
+308
| best_schedule = | ||
| current_best.get_makespan() < best_schedule.get_makespan() | ||
| ? current_best | ||
| : best_schedule; |
There was a problem hiding this comment.
Shouldn't this be somehow handled by the omp reduction? (I don't know this functionality)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a naive port of our
milqscheduler to C++.There are still multiple
TODOs to fix,and probably tons of performance improvements.The build seems fine @kayaercument how are we supposed to include the QRM?
Also, some things are missing compared to the QRM setup, like git hooks.
Some more todos: