dpl: make optimize_mirroring deterministic

[oharboe]

net_box_map_ is a std::unordered_map<dbNet*, NetBox>, so the
sorted_boxes vector built from it starts in heap-address order. The
comparator keys on hpwl() alone and std::ranges::sort is not stable,
so equal-hpwl boxes keep that non-deterministic order. The greedy
mirrorCandidates() then mutates boxes in that order, so the mirroring
result — and the written db — differs run to run.

Tie-break the sort by net id for a stable total order.

src/dpl/test mirror1/2/3 output is unchanged.

[gemini-code-assist]

Code Review

This pull request updates the sorting logic for net boxes in OptimizeMirroring::run by introducing a tie-breaker based on the net ID when the HPWL values are equal, ensuring stable and deterministic output. There are no review comments, and I have no feedback to provide.

[oharboe]

While here, I swept the codebase for the same shape: a non-stable sort whose comparator is not a strict total order (single key, ties possible), over a sequence whose order is itself non-deterministic — built by iterating a pointer-keyed unordered_map/unordered_set, or tie-broken on a raw pointer. Most sort sites are fine (they key on a unique id/name, or the input is an ordered container). The ones below are not.

Likely bugs

rsz — buffer_fast_sizes_ is a std::unordered_set<LibertyCell*>; it is copied into buffer_sizes_ and sorted by input capacitance only. A greedy walk then takes the first cell above a threshold, so among equal-Cin cells the choice is heap-order-dependent and the written netlist varies run to run.

OpenROAD/src/rsz/src/RepairDesign.cc

Lines 648 to 651 in 73ab739

	std::ranges::sort(buffer_sizes_,
	[=](sta::LibertyCell* a, sta::LibertyCell* b) {
	return bufferCin(a) < bufferCin(b);
	});

OpenROAD/src/rsz/src/Rebuffer.cc

Lines 1502 to 1504 in 73ab739

	std::ranges::sort(buffer_sizes_, [=](BufferSize a, BufferSize b) {
	return bufferCin(a.cell) < bufferCin(b.cell);
	});

odb DEF writer — obstruction sort has a copy/paste typo: layer_b is computed from bbox_a (L1141), so the layer key is dead and equal-rect obstructions on different layers reorder in the emitted DEF.

OpenROAD/src/odb/src/defout/defout_impl.cpp

Lines 1136 to 1147 in 73ab739

	std::ranges::sort(sorted_obs, [](dbObstruction* a, dbObstruction* b) {
	dbBox* bbox_a = a->getBBox();
	dbTechLayer* layer_a = bbox_a->getTechLayer();
	dbBox* bbox_b = b->getBBox();
	dbTechLayer* layer_b = bbox_a->getTechLayer();
	if (layer_a != layer_b) {
	return layer_a->getNumber() < layer_b->getNumber();
	}
	Rect rect_a = bbox_a->getBox();
	Rect rect_b = bbox_b->getBox();

Suspects (single-key comparator feeding output; only deterministic if the input source happens to be ordered today)

DEF blockage sort — keyed on rect only, no unique tie-break:

OpenROAD/src/odb/src/defout/defout_impl.cpp

Lines 1203 to 1207 in 73ab739

	std::ranges::sort(sorted_blockages, [](dbBlockage* a, dbBlockage* b) {
	dbBox* bbox_a = a->getBBox();
	dbBox* bbox_b = b->getBBox();
	Rect rect_a = bbox_a->getBox();
	Rect rect_b = bbox_b->getBox();

DEF sortedSet — keyed on name only (relies on names being unique):

OpenROAD/src/odb/src/defout/defout_impl.cpp

Line 71 in 73ab739

std::sort(sorted.begin(), sorted.end(), [](T* a, T* b) {

dft scan replacement — candidates gathered from an unordered_set<LibertyCell*>, sorted by |Δ drive resistance| only, then .at(0) is substituted into the netlist:

OpenROAD/src/dft/src/replace/ScanReplace.cpp

Lines 179 to 190 in 73ab739

	std::ranges::sort(
	scan_candidates, [&non_scan_cell](const auto& lhs, const auto& rhs) {
	// We want to keep the difference as close as possible to
	// the non_scan_cell
	const double difference_lhs
	= DifferencePerformanceCells(non_scan_cell, lhs->getScanCell());
	const double difference_rhs
	= DifferencePerformanceCells(non_scan_cell, rhs->getScanCell());
	return difference_lhs < difference_rhs;
	});
	return std::move(scan_candidates.at(0));

cts dummy-cell pick — sorted by input cap only; source is STA library iteration:

OpenROAD/src/cts/src/TritonCTS.cpp

Lines 2466 to 2470 in 73ab739

	std::ranges::sort(
	dummyCandidates,
	[](const sta::LibertyCell* cell1, const sta::LibertyCell* cell2) {
	return (getInputCap(cell1) < getInputCap(cell2));
	});

grt congested-NDR-net order — sorted by edge count only; feeds rip-up/reroute selection:

OpenROAD/src/grt/src/fastroute/src/graph2d.cpp

Lines 549 to 551 in 73ab739

	void Graph2D::sortCongestedNDRnets()
	{
	std::ranges::sort(congested_ndrs_, NDRCongestionComparator());

par port sort — std::ranges::sort on vector<pair<string, PortDirection*>> falls back to comparing the PortDirection* pointer on equal names:

OpenROAD/src/par/src/PartitionMgr.cpp

Line 744 in 73ab739

std::ranges::sort(ports);

For reference, dbNetwork.cc is the correct pattern — same unordered_set<dbNet*> source, made deterministic by sorting on the unique getId():

OpenROAD/src/dbSta/src/dbNetwork.cc

Lines 419 to 422 in 73ab739

	flat_nets_vec_.assign(flat_nets_set.begin(), flat_nets_set.end());
	// Sort these nets so their order is determinisitc
	std::ranges::sort(
	flat_nets_vec_, {}, [](auto const* net1) { return net1->getId(); });

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates the sorting logic for sorted_boxes in OptimizeMirroring::run within src/dpl/src/OptMirror.cpp. It introduces a tie-breaker using the net ID when the HPWL values are equal, ensuring stable sorting output since the underlying map is unordered. There are no review comments, so I have no feedback to provide.

[gadfort]

you might want to do:

const auto hpwl1 = net_box1->hpwl();
const auto hpwl2 = net_box2->hpwl();
if (hpwl1  != hpwl2 ) {
  return hpwl1 > hpwl2;
}

To avoid recomputing the hpwl twice