Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heuristic #9

Draft
wants to merge 53 commits into
base: main
Choose a base branch
from
Draft

Heuristic #9

wants to merge 53 commits into from

Conversation

Gistbatch
Copy link
Contributor

@Gistbatch Gistbatch commented Feb 6, 2024

I've added a scatter search heuristic for finding a schedule.
Todos:

  • Integrate into existing API
  • Benchmark
  • Parallelize
  • Test
  • Remove/decrease randomness from test

For more todos, see generate_heuristic_info_schedule

@Gistbatch Gistbatch self-assigned this Feb 6, 2024
@Gistbatch Gistbatch requested a review from Ectras February 6, 2024 15:26
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this belongs to the RL agent and therefore shouldn't be part of this branch / PR

from ..bin_schedule import _do_bin_pack


class Option(Protocol):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to PartitioningScheme or InitialPartitioningScheme

"""

schedules = []
num_cores = max(len(OPTIONS), cpu_count())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a min? Otherwise, using more cores than physically available has probably rather negative impacts on performance.

Comment on lines +271 to +274
if "partition_size" not in kwargs:
partition_size = 10
else:
partition_size = kwargs["partition_size"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if "partition_size" not in kwargs:
partition_size = 10
else:
partition_size = kwargs["partition_size"]
partition_size = kwargs.pop("partition_size", 10)

or equivalently with get if you don't want to mutate the kwargs

Comment on lines +282 to +284
if partition[-1] == 1:
partition[-1] = 2
partition[-2] -= 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add an assertion that partition[-2] > 2 before subtracting from it

Comment on lines +77 to +83
def is_feasible(schedule: Schedule) -> bool:
"""Checks if a schedule is feasible."""
return all(
sum(job.circuit.num_qubits for job in bucket.jobs) <= machine.capacity
for machine in schedule.machines
for bucket in machine.buckets
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not implement this in the Schedule class?

]
benchmark_results: list[Result] = []
for benchmark in benchmarks:
problme_circuits = _cut_circuits(benchmark, setting)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

problem_circuits

return results


def _cut_circuits(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is significant code duplication (_cut_circuits, _generate_partitions and _partition_big_to_small) are similar in initialize.py

for machine in schedule.machines:
if len(machine.buckets) == 0:
continue
swap_buckets = np.random.randint(1, dtype=bool)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense that all swaps are done either on bucket or on job level? In other words, wouldn't it make sense to randomly assign swap_buckets again in each iteration of for _ in range(number_of_swaps)?


def _diversify(population: list[Schedule]) -> list[Schedule]:
"""Swaps jobs between different machines."""
local_population = population.copy()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local_population is not used. Instead, you are editing the passed population (which still kind of works because local_population is only a shallow copy)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants