From dc68d38293774f79d630cd6e1e2b2e82b72c4f69 Mon Sep 17 00:00:00 2001 From: EstherLerouzic Date: Fri, 2 Jun 2023 16:50:50 +0200 Subject: [PATCH] fix ila names auto design were changed long ago and these functions did not apply the changes. Besides there was a confusion between request_element class where loose is a string, and PathRequest from topology.requests where loose_list is a list of strings. This patch corrects the naming and also the tests, because it used the wrong class to gererate xls services Signed-off-by: EstherLerouzic Change-Id: I564b77576459d6cb47767398a2db8138ba6ad1e4 --- gnpy/tools/convert.py | 68 ++++++++++++++++++----------- gnpy/tools/service_sheet.py | 86 +++++++++++++++++-------------------- tests/test_parser.py | 76 +++++++++++++------------------- 3 files changed, 113 insertions(+), 117 deletions(-) diff --git a/gnpy/tools/convert.py b/gnpy/tools/convert.py index aee2b9d80..bf25a8e2a 100755 --- a/gnpy/tools/convert.py +++ b/gnpy/tools/convert.py @@ -575,24 +575,41 @@ def corresp_names(input_filename, network): corresp_ila[my_e.from_city] = [name] # complete with potential autodesign names: amplifiers for my_l in links: - name = f'Edfa0_fiber ({my_l.to_city} \u2192 {my_l.from_city})-{my_l.west_cable}' - if name in ila: - if my_l.from_city in corresp_ila.keys(): - # "east edfa in Stbrieuc to Rennes_STA" is equivalent name as - # "Edfa0_fiber (Lannion_CAS → Stbrieuc)-F056" - # "west edfa in Stbrieuc to Rennes_STA" is equivalent name as - # "Edfa0_fiber (Rennes_STA → Stbrieuc)-F057" - # does not filter names: all types (except boosters) are created. - # in case fibers are splitted the name here is a prefix - corresp_ila[my_l.from_city].append(name) - else: - corresp_ila[my_l.from_city] = [name] - name = f'Edfa0_fiber ({my_l.from_city} \u2192 {my_l.to_city})-{my_l.east_cable}' - if name in ila: - if my_l.to_city in corresp_ila.keys(): - corresp_ila[my_l.to_city].append(name) - else: - corresp_ila[my_l.to_city] = [name] + # create names whatever the type and filter them out + names = [f'Edfa_preamp_roadm {my_l.from_city}_from_fiber ({my_l.to_city} \u2192 {my_l.from_city})-{my_l.west_cable}', + f'Edfa_booster_roadm {my_l.from_city}_to_fiber ({my_l.from_city} \u2192 {my_l.to_city})-{my_l.east_cable}'] + for name in names: + if name in ila: + if my_l.from_city in corresp_ila.keys(): + # "east edfa in Stbrieuc to Rennes_STA" is equivalent name as + # "Edfa_booster_roadm Stbrieuc_to_fiber (Lannion_CAS → Stbrieuc)-F056" + # "west edfa in Stbrieuc to Rennes_STA" is equivalent name as + # "Edfa_preamp_roadm Stbrieuc_to_fiber (Rennes_STA → Stbrieuc)-F057" + # in case fibers are splitted the name here is a prefix + corresp_ila[my_l.from_city].append(name) + else: + corresp_ila[my_l.from_city] = [name] + names = [f'Edfa_preamp_roadm {my_l.to_city}_from_fiber ({my_l.from_city} \u2192 {my_l.to_city})-{my_l.east_cable}', + f'Edfa_booster_roadm {my_l.to_city}_to_fiber ({my_l.to_city} \u2192 {my_l.from_city})-{my_l.west_cable}'] + for name in names: + if name in ila: + if my_l.to_city in corresp_ila.keys(): + corresp_ila[my_l.to_city].append(name) + else: + corresp_ila[my_l.to_city] = [name] + + for node in nodes: + names = [f'east edfa in {node.city}', f'west edfa in {node.city}'] + for name in names: + if name in ila: + if node.city in corresp_ila.keys(): + # "east edfa in Stbrieuc to Rennes_STA" (created with Eqpt) is equivalent name as + # "east edfa in Stbrieuc" or "west edfa in Stbrieuc" (created with Links sheet) + # depending on link node order + corresp_ila[node.city].append(name) + else: + corresp_ila[node.city] = [name] + # merge fused with ila: for key, val in corresp_fused.items(): if key in corresp_ila.keys(): @@ -781,16 +798,19 @@ def corresp_next_node(network, corresp_ila, corresp_roadm): user ILA name Stbrieuc covers the two direction. convert.py creates 2 different ILA with possible names (depending on the direction and if the eqpt was defined in eqpt sheet) + for an ILA and if it is defined in eqpt: - east edfa in Stbrieuc to Rennes_STA - west edfa in Stbrieuc to Rennes_STA - - Edfa0_fiber (Lannion_CAS → Stbrieuc)-F056 - - Edfa0_fiber (Rennes_STA → Stbrieuc)-F057 + for an ILA and if it is not defined in eqpt: + - east edfa in Stbrieuc + - west edfa in Stbrieuc + for a roadm + "Edfa_preamp_roadm node1_from_fiber (siteE → node1)-CABLES#19" + "Edfa_booster_roadm node1_to_fiber (node1 → siteE)-CABLES#19" next_nodes finds the user defined name of next node to be able to map the path constraints - east edfa in Stbrieuc to Rennes_STA next node = Rennes_STA - west edfa in Stbrieuc to Rennes_STA next node Lannion_CAS - Edfa0_fiber (Lannion_CAS → Stbrieuc)-F056 and Edfa0_fiber (Rennes_STA → Stbrieuc)-F057 - do not exist the function supports fiber splitting, fused nodes and shall only be called if excel format is used for both network and service """ @@ -801,8 +821,8 @@ def corresp_next_node(network, corresp_ila, corresp_roadm): for ila_elem in ila_list: # find the node with ila_elem string _in_ the node uid. 'in' is used instead of # '==' to find composed nodes due to fiber splitting in autodesign. - # eg if elem_ila is 'Edfa0_fiber (Lannion_CAS → Stbrieuc)-F056', - # node uid 'Edfa0_fiber (Lannion_CAS → Stbrieuc)-F056_(1/2)' is possible + # eg if elem_ila is 'east edfa in Stbrieuc to Rennes_STA', + # node uid 'east edfa in Stbrieuc to Rennes_STA-_(1/2)' is possible correct_ila_name = next(n.uid for n in network.nodes() if ila_elem in n.uid) temp.remove(ila_elem) temp.append(correct_ila_name) diff --git a/gnpy/tools/service_sheet.py b/gnpy/tools/service_sheet.py index 7952af90a..26208e9b2 100644 --- a/gnpy/tools/service_sheet.py +++ b/gnpy/tools/service_sheet.py @@ -280,10 +280,8 @@ def correct_xls_route_list(network_filename, network, pathreqlist): # and last elem in the constraints respectively. Other positions must lead to an error # caught later on if pathreq.nodes_list and pathreq.source == pathreq.nodes_list[0]: - pathreq.loose_list.pop(0) pathreq.nodes_list.pop(0) if pathreq.nodes_list and pathreq.destination == pathreq.nodes_list[-1]: - pathreq.loose_list.pop(-1) pathreq.nodes_list.pop(-1) # Then process user defined constraints with respect to automatic namings temp = deepcopy(pathreq) @@ -309,54 +307,50 @@ def correct_xls_route_list(network_filename, network, pathreqlist): nodes_suggestion = corresp_ila[n_id] else: nodes_suggestion = [] - if nodes_suggestion: - try: - if len(nodes_suggestion) > 1: - # if there is more than one suggestion, we need to choose the direction - # we rely on the next node provided by the user for this purpose - new_n = next(n for n in nodes_suggestion - if n in next_node.keys() and next_node[n] - in temp.nodes_list[i:] + [pathreq.destination] and - next_node[n] not in temp.nodes_list[:i]) - else: - new_n = nodes_suggestion[0] - if new_n != n_id: - # warns the user when the correct name is used only in verbose mode, - # eg 'a' is a roadm and correct name is 'roadm a' or when there was - # too much ambiguity, 'b' is an ila, its name can be: - # Edfa0_fiber (a → b)-xx if next node is c or - # Edfa0_fiber (c → b)-xx if next node is a - msg = f'Request {pathreq.request_id}: Invalid route node specified:' \ - + f'\n\t\'{n_id}\', replaced with \'{new_n}\'' - logger.warning(msg) - pathreq.nodes_list[pathreq.nodes_list.index(n_id)] = new_n - except StopIteration: - # shall not come in this case, unless requested direction does not exist - msg = f'Request {pathreq.request_id}: Invalid route specified {n_id}: could' \ - + ' not decide on direction, skipped!.\nPlease add a valid' \ - + ' direction in constraints (next neighbour node)' - logger.warning(msg) - pathreq.loose_list.pop(pathreq.nodes_list.index(n_id)) - pathreq.nodes_list.remove(n_id) - else: - if temp.loose_list[i] == 'LOOSE': - # if no matching can be found in the network just ignore this constraint - # if it is a loose constraint - # warns the user that this node is not part of the topology - msg = f'Request {pathreq.request_id}: Invalid node specified:\n\t\'{n_id}\'' \ - + ', could not use it as constraint, skipped!' - logger.warning(msg) - pathreq.loose_list.pop(pathreq.nodes_list.index(n_id)) - pathreq.nodes_list.remove(n_id) + try: + if len(nodes_suggestion) > 1: + # if there is more than one suggestion, we need to choose the direction + # we rely on the next node provided by the user for this purpose + new_n = next(n for n in nodes_suggestion + if n in next_node.keys() and next_node[n] + in temp.nodes_list[i:] + [pathreq.destination] and + next_node[n] not in temp.nodes_list[:i]) + elif len(nodes_suggestion) == 1: + new_n = nodes_suggestion[0] else: - msg = f'Request {pathreq.request_id}: Could not find node:\n\t\'{n_id}\' in network' \ + if temp.loose == 'LOOSE': + # if no matching can be found in the network just ignore this constraint + # if it is a loose constraint + # warns the user that this node is not part of the topology + msg = f'{pathreq.request_id}: Invalid node specified:\n\t\'{n_id}\', ' \ + + 'could not use it as constraint, skipped!' + logger.warning(msg) + pathreq.nodes_list.remove(n_id) + continue + msg = f'{pathreq.request_id}: Could not find node:\n\t\'{n_id}\' in network' \ + ' topology. Strict constraint can not be applied.' + logger.critical(msg) raise ServiceError(msg) + if new_n != n_id: + # warns the user when the correct name is used only in verbose mode, + # eg 'a' is a roadm and correct name is 'roadm a' or when there was + # too much ambiguity, 'b' is an ila, its name can be: + # "east edfa in b to c", or "west edfa in b to a" if next node is c or + # "west edfa in b to c", or "east edfa in b to a" if next node is a + msg = f'Request {pathreq.request_id}: Invalid route node specified:' \ + + f'\n\t\'{n_id}\', replaced with \'{new_n}\'' + pathreq.nodes_list[pathreq.nodes_list.index(n_id)] = new_n + except StopIteration: + # shall not come in this case, unless requested direction does not exist + msg = f'Request {pathreq.request_id}: Invalid route specified {n_id}: could' \ + + ' not decide on direction, skipped!.\nPlease add a valid' \ + + ' direction in constraints (next neighbour node)' + pathreq.nodes_list.remove(n_id) else: - if temp.loose_list[i] == 'LOOSE': - logger.warning(f'Request {pathreq.request_id}: Invalid route node specified:\n\t\'{n_id}\'' - + ' type is not supported as constraint with xls network input, skipped!') - pathreq.loose_list.pop(pathreq.nodes_list.index(n_id)) + if temp.loose == 'LOOSE': + logger.warning('Invalid route node specified:\n\t\'%s\'' + + ' type is not supported as constraint with xls network input,' + + ' skipped!', n_id) pathreq.nodes_list.remove(n_id) else: msg = f'Invalid route node specified \n\t\'{n_id}\'' \ diff --git a/tests/test_parser.py b/tests/test_parser.py index af309916b..9160b3fd5 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -32,7 +32,7 @@ from gnpy.tools.convert import convert_file from gnpy.tools.json_io import (load_json, load_network, save_network, load_equipment, requests_from_json, disjunctions_from_json, network_to_json, network_from_json) -from gnpy.tools.service_sheet import read_service_sheet, correct_xls_route_list +from gnpy.tools.service_sheet import read_service_sheet, correct_xls_route_list, Request_element, Request TEST_DIR = Path(__file__).parent DATA_DIR = TEST_DIR / 'data' @@ -284,44 +284,43 @@ def test_json_response_generation(xls_input, expected_response_file): @pytest.mark.parametrize('source, destination, route_list, hoptype, expected_correction', [ - ('trx Brest_KLA', 'trx Vannes_KBE', + ('Brest_KLA', 'Vannes_KBE', 'roadm Brest_KLA | roadm Lannion_CAS | roadm Lorient_KMA | roadm Vannes_KBE', - 'STRICT', + 'no', ['roadm Brest_KLA', 'roadm Lannion_CAS', 'roadm Lorient_KMA', 'roadm Vannes_KBE']), - ('trx Brest_KLA', 'trx Vannes_KBE', + ('Brest_KLA', 'Vannes_KBE', 'trx Brest_KLA | roadm Lannion_CAS | roadm Lorient_KMA | roadm Vannes_KBE', - 'STRICT', + 'No', ['roadm Lannion_CAS', 'roadm Lorient_KMA', 'roadm Vannes_KBE']), - ('trx Lannion_CAS', 'trx Rennes_STA', 'trx Rennes_STA', 'LOOSE', []), - ('trx Lannion_CAS', 'trx Lorient_KMA', 'toto', 'LOOSE', []), - ('trx Lannion_CAS', 'trx Lorient_KMA', 'toto', 'STRICT', 'Fail'), - ('trx Lannion_CAS', 'trx Lorient_KMA', 'Corlay | Loudeac | Lorient_KMA', 'LOOSE', + ('Lannion_CAS', 'Rennes_STA', 'trx Rennes_STA', 'yes', []), + ('Lannion_CAS', 'Lorient_KMA', 'toto', 'yes', []), + ('Lannion_CAS', 'Lorient_KMA', 'toto', 'no', 'Fail'), + ('Lannion_CAS', 'Lorient_KMA', 'Corlay | Loudeac | Lorient_KMA', 'yes', ['west fused spans in Corlay', 'west fused spans in Loudeac', 'roadm Lorient_KMA']), - ('trx Lannion_CAS', 'trx Lorient_KMA', 'Ploermel | Vannes_KBE', 'LOOSE', + ('Lannion_CAS', 'Lorient_KMA', 'Ploermel | Vannes_KBE', 'yes', ['east edfa in Ploermel to Vannes_KBE', 'roadm Vannes_KBE']), - ('trx Rennes_STA', 'trx Brest_KLA', 'Vannes_KBE | Quimper | Brest_KLA', 'LOOSE', + ('Rennes_STA', 'Brest_KLA', 'Vannes_KBE | Quimper | Brest_KLA', 'yes', ['roadm Vannes_KBE', 'west edfa in Quimper to Lorient_KMA', 'roadm Brest_KLA']), - ('trx Brest_KLA', 'trx Rennes_STA', 'Brest_KLA | Quimper | Lorient_KMA', 'LOOSE', + ('Brest_KLA', 'Rennes_STA', 'Brest_KLA | Quimper | Lorient_KMA', 'yes', ['roadm Brest_KLA', 'east edfa in Quimper to Lorient_KMA', 'roadm Lorient_KMA']), - ('Brest_KLA', 'trx Rennes_STA', '', 'LOOSE', 'Fail'), - ('trx Brest_KLA', 'Rennes_STA', '', 'LOOSE', 'Fail'), - ('Brest_KLA', 'Rennes_STA', '', 'LOOSE', 'Fail'), - ('Brest_KLA', 'trx Rennes_STA', '', 'STRICT', 'Fail'), - ('trx Brest_KLA', 'trx Rennes_STA', 'trx Rennes_STA', 'STRICT', []), - ('trx Brest_KLA', 'trx Rennes_STA', None, '', []), - ('trx Brest_KLA', 'trx Rennes_STA', 'Brest_KLA | Quimper | Ploermel', 'LOOSE', + ('Brest_KLA', 'trx Rennes_STA', '', 'yes', 'Fail'), + ('trx Brest_KLA', 'Rennes_STA', '', 'yes', 'Fail'), + ('trx Brest_KLA', 'trx Rennes_STA', '', 'yes', 'Fail'), + ('Brest_KLA', 'trx Rennes_STA', '', 'no', 'Fail'), + ('Brest_KLA', 'Rennes_STA', 'trx Rennes_STA', 'no', []), + ('Brest_KLA', 'Rennes_STA', None, '', []), + ('Brest_KLA', 'Rennes_STA', 'Brest_KLA | Quimper | Ploermel', 'yes', ['roadm Brest_KLA']), - ('trx Brest_KLA', 'trx Rennes_STA', 'Brest_KLA | Quimper | Ploermel', 'STRICT', + ('Brest_KLA', 'Rennes_STA', 'Brest_KLA | Quimper | Ploermel', 'no', ['roadm Brest_KLA']), - ('trx Brest_KLA', 'trx Rennes_STA', 'Brest_KLA | trx Quimper', 'LOOSE', ['roadm Brest_KLA']), - ('trx Brest_KLA', 'trx Rennes_STA', 'Brest_KLA | trx Lannion_CAS', 'LOOSE', ['roadm Brest_KLA']), - ('trx Brest_KLA', 'trx Rennes_STA', 'Brest_KLA | trx Lannion_CAS', 'STRICT', 'Fail') + ('Brest_KLA', 'Rennes_STA', 'Brest_KLA | trx Quimper', 'yes', ['roadm Brest_KLA']), + ('Brest_KLA', 'Rennes_STA', 'Brest_KLA | trx Lannion_CAS', 'yes', ['roadm Brest_KLA']), + ('Brest_KLA', 'Rennes_STA', 'Brest_KLA | trx Lannion_CAS', 'no', 'Fail') ]) def test_excel_ila_constraints(source, destination, route_list, hoptype, expected_correction): """add different kind of constraints to test all correct_route cases""" service_xls_input = DATA_DIR / 'testTopology.xls' network_json_input = DATA_DIR / 'testTopology_auto_design_expected.json' - equipment = load_equipment(eqpt_filename) network = load_network(network_json_input, equipment) # increase length of one span to trigger automatic fiber splitting included by autodesign # so that the test also covers this case @@ -331,37 +330,20 @@ def test_excel_ila_constraints(source, destination, route_list, hoptype, expecte p_db = default_si.power_dbm p_total_db = p_db + lin2db(automatic_nch(default_si.f_min, default_si.f_max, default_si.spacing)) build_network(network, equipment, p_db, p_total_db) - # create params for a request based on input - nodes_list = route_list.split(' | ') if route_list is not None else [] + # create params for a xls request based on input (note that this not the same type as PathRequest) params = { 'request_id': '0', 'source': source, - 'bidir': False, 'destination': destination, - 'trx_type': '', - 'trx_mode': '', - 'format': '', - 'spacing': '', - 'nodes_list': nodes_list, - 'loose_list': [hoptype for node in nodes_list] if route_list is not None else '', - 'f_min': 0, - 'f_max': 0, - 'baud_rate': 0, - 'OSNR': None, - 'bit_rate': None, - 'cost': None, - 'roll_off': 0, - 'tx_osnr': 0, - 'tx_power': 0, - 'penalties': None, - 'min_spacing': None, + 'trx_type': 'Voyager', + 'spacing': 50, + 'nodes_list': route_list, + 'is_loose': hoptype, 'nb_channel': 0, 'power': 0, 'path_bandwidth': 0, - 'effective_freq_slot': None, - 'equalization_offset_db': 0 } - request = PathRequest(**params) + request = Request_element(Request(**params), equipment, False) if expected_correction != 'Fail': [request] = correct_xls_route_list(service_xls_input, network, [request])