From 26e0a6c636cffb0dbe97b26d1c4ecc76fa435045 Mon Sep 17 00:00:00 2001 From: Robert Ricci Date: Wed, 10 Nov 2010 01:28:50 -0700 Subject: [PATCH] Re-think no_connect violations Re-think a very old idea in assign - that no_connect violations for unmapped links should not be assessed against links whose endpoints are not mapped. The problem with this is that assign is too hesitant to map nodes - if there is more than one link that can't be mapped or whose other end isn't mapped yet, then assign thinks it's better to leave the node unmapped. This leads to confusing solutions (in which nodes remain unmapped when really it's the links that are the problem), and I beleive it keeps assign from exploring valuable parts of the solution space. This patch chanes the behavior, so that no_connect violations get scored even when the endpoints of the link are not mapped. Not well enough tested to run in production. --- assign/anneal.cc | 9 +++-- assign/parse_request_rspec.cc | 2 +- assign/parse_top.cc | 2 +- assign/score.cc | 73 +++++++++++++++++++++++------------ assign/score.h | 1 + 5 files changed, 57 insertions(+), 30 deletions(-) diff --git a/assign/anneal.cc b/assign/anneal.cc index f7283e0f2..40c63d6ef 100644 --- a/assign/anneal.cc +++ b/assign/anneal.cc @@ -747,7 +747,7 @@ void anneal(bool scoring_selftest, bool check_fixed_nodes, } if ((oldscore != get_score()) || (oldviolated != violated)) { cerr << "Scoring problem adding a mapping - oldscore was " << - oldscore << " newscore is " << newscore << " tempscore was " + oldscore << " current score is " << get_score() << " tempscore was " << tempscore << endl; cerr << "oldviolated was " << oldviolated << " newviolated is " << violated << " tempviolated was " << tempviolated << endl; @@ -1321,6 +1321,7 @@ NOTQUITEDONE: * Okay, now that we've jumped through enough hoops, we can actually * do the scoring */ + mark_vlink_assigned(vlink); score_link_info(*vedge_it, src_pnode, dst_pnode, src_vnode, dst_vnode); } else { /* @@ -1329,9 +1330,9 @@ NOTQUITEDONE: * mapped, then we have to make sure the score reflects that. */ if (!dst_vnode->assigned || !src_vnode->assigned) { - vlink->link_info.type_used = tb_link_info::LINK_UNMAPPED; - } else { - mark_vlink_unassigned(vlink); + if (!vlink->no_connection) { + mark_vlink_unassigned(vlink); + } } } } diff --git a/assign/parse_request_rspec.cc b/assign/parse_request_rspec.cc index 100eace5d..dd829a4dd 100644 --- a/assign/parse_request_rspec.cc +++ b/assign/parse_request_rspec.cc @@ -699,7 +699,7 @@ bool populate_link (DOMElement* elt, virt_link->emulated = emulated; virt_link->allow_delayed = allow_delayed; virt_link->allow_trivial = allow_trivial; - virt_link->no_connection = false; + virt_link->no_connection = true; virt_link->delay_info.bandwidth = bandwidth; virt_link->delay_info.delay = latency; virt_link->delay_info.loss = packetLoss; diff --git a/assign/parse_top.cc b/assign/parse_top.cc index d7aa19019..02c8aaf84 100644 --- a/assign/parse_top.cc +++ b/assign/parse_top.cc @@ -266,7 +266,7 @@ int parse_top(tb_vgraph &vg, istream& input) (sscanf(lossweight.c_str(),"%lg",&(l->delay_info.loss_weight)) != 1)) { top_error("Bad line line, bad delay characteristics."); } - l->no_connection = false; + l->no_connection = true; l->name = name; l->allow_delayed = true; #ifdef ALLOW_TRIVIAL_DEFAULT diff --git a/assign/score.cc b/assign/score.cc index ea54711ae..34e0557cb 100644 --- a/assign/score.cc +++ b/assign/score.cc @@ -183,8 +183,11 @@ void init_score() tie(vedge_it,end_vedge_it) = edges(VG); for (;vedge_it!=end_vedge_it;++vedge_it) { tb_vlink *vlink=get(vedge_pmap,*vedge_it); - vlink->link_info.type_used=tb_link_info::LINK_UNMAPPED; - vlink->no_connection=false; + SADD(SCORE_NO_CONNECTION); + vlink->no_connection=true; + vinfo.no_connection++; + vlink->link_info.type_used = tb_link_info::LINK_UNMAPPED; + violated++; } pvertex_iterator pvertex_it,end_pvertex_it; tie(pvertex_it,end_pvertex_it) = vertices(PG); @@ -509,6 +512,8 @@ inline float resolution_cost(tb_link_info::linkType res_type) { * Mark a vlink as unassigned */ void mark_vlink_unassigned(tb_vlink *vlink) { + SDEBUG(cerr << " marking " << vlink->name << " unassigned" << endl;) + assert(!vlink->no_connection); SADD(SCORE_NO_CONNECTION); vlink->no_connection=true; vinfo.no_connection++; @@ -516,6 +521,18 @@ void mark_vlink_unassigned(tb_vlink *vlink) { violated++; } +/* + * Mark a vlink as assigned: fix up violations + */ +void mark_vlink_assigned(tb_vlink *vlink) { + SDEBUG(cerr << " marking " << vlink->name << " assigned" << endl;) + assert(vlink->no_connection); + SSUB(SCORE_NO_CONNECTION); + vlink->no_connection=false; + vinfo.no_connection--; + violated--; +} + /* * Resolve an individual vlink */ @@ -570,9 +587,16 @@ void resolve_link(vvertex vv, pvertex pv, tb_vnode *vnode, tb_pnode *pnode, * Note: We can't get here when doing adjust_to_native_bandwidth, * since it's illegal to allow trivial links when it's in use. */ + SDEBUG(cerr << " allowed" << endl;) + if (vlink->no_connection) { + mark_vlink_assigned(vlink); + } score_link_info(edge,pnode,dest_pnode,vnode,dest_vnode); } else { - mark_vlink_unassigned(vlink); + SDEBUG(cerr << " not allowed" << endl;) + if (!vlink->no_connection) { + mark_vlink_unassigned(vlink); + } } } else { //assert(resolution_index <= 1) @@ -589,18 +613,10 @@ void resolve_link(vvertex vv, pvertex pv, tb_vnode *vnode, tb_pnode *pnode, */ if (resolution_index == 0) { SDEBUG(cerr << " Could not find any resolutions" << endl;) - mark_vlink_unassigned(vlink); - } else { - /* - * Check to see if we are fixing a violation - */ - if (vlink->no_connection) { - SDEBUG(cerr << " Fixing previous violations." << endl); - SSUB(SCORE_NO_CONNECTION); - vlink->no_connection=false; - vinfo.no_connection--; - violated--; + if (!vlink->no_connection) { + mark_vlink_unassigned(vlink); } + } else { /* * Choose a link */ @@ -658,9 +674,20 @@ void resolve_link(vvertex vv, pvertex pv, tb_vnode *vnode, tb_pnode *pnode, #ifdef PENALIZE_UNUSED_INTERFACES pnode->used_interfaces++; #endif + /* + * Make it so + */ vlink->link_info = resolutions[index]; SDEBUG(cerr << " choice:" << vlink->link_info); score_link_info(edge,pnode,dest_pnode,vnode,dest_vnode); + + /* + * Check to see if we are fixing a violation + */ + if (vlink->no_connection) { + SDEBUG(cerr << " Fixing previous violations." << endl); + mark_vlink_assigned(vlink); + } } } } @@ -1002,16 +1029,7 @@ void remove_node(vvertex vv) } } - // A 'not-connected' vlink only counts as a violation if both of its - // endpoints are assigned - if (vlink->no_connection) { - SDEBUG(cerr << " link no longer in violation.\n";) - SSUB(SCORE_NO_CONNECTION); - vlink->no_connection=false; - vinfo.no_connection--; - violated--; - } - + // Only unscore the link if the vnode on the other end is assigned - this // way, only the first end to be unmapped causes unscoring if (! dest_vnode->assigned) { @@ -1022,6 +1040,13 @@ void remove_node(vvertex vv) pvertex dest_pv = dest_vnode->assignment; tb_pnode *dest_pnode = get(pvertex_pmap,dest_pv); unscore_link_info(*vedge_it,pnode,dest_pnode,vnode,dest_vnode); + + // If the other end was connected before, it's not now + if (!vlink->no_connection) { + SDEBUG(cerr << " link now in violation.\n";) + mark_vlink_unassigned(vlink); + } + } #ifdef PENALIZE_UNUSED_INTERFACES diff --git a/assign/score.h b/assign/score.h index d628d02c1..c9a238575 100644 --- a/assign/score.h +++ b/assign/score.h @@ -97,6 +97,7 @@ void resolve_link(vvertex vv, pvertex pv, tb_vnode *vnode, tb_pnode *pnode, void resolve_links(vvertex vv, pvertex pv, tb_vnode *vnode, tb_pnode *pnode, bool deterministic); void mark_vlink_unassigned(tb_vlink *vlink); +void mark_vlink_assigned(tb_vlink *vlink); /* * Declaration of many of the variables used for scoring. Default values -- GitLab