See GitHub issue #391
There has been a change in the networkx drawing API, which means
'figure' is no longer an accepted keyword argument.
Change-Id: I8600e8cd5eb2cb378a529c7857f664c1ebed8337
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
The old code assumed that the Fused node only connects Fiber nodes. In a
sequence of Fused - Amplifier - Fused - Fiber, the Amplifier would be
included by a mistake. In addition, the code was not that easy to read,
and it just instantiated StopIteration without raising that (which would
be an error in a generator context). It was also rather strict, failing
if the iterator was requested for an "edge node" (a transponder), and
one of the exceptions was not actually an f-string.
Finally, the span_loss function would occasionally report wrong values
(for example in the provided test case, span_loss("fused7") would say 1
instead of 17).
Fix this by making sure that prev_node_generator() and
next_node_generator() never return anything but Fiber and Fused nodes,
which made it possible to simplify the span_loss() function. This should
now properly account for the accumulated losses of an arbitrary sequence
of Fiber and Fused elements.
I went over this a few times because set_egress_amplifier() calls
span_loss() on a *ROADM* node type. That does not make any sense, and
the code deals with this "properly" by returning a loss of 0 dB. It was
a bit confusing for me to see that it's actually OK to ask for a "span
loss" that's identified by a ROADM.
A side effect of this code is that Fused instances that are isolated
from the rest of the topology no longer raise an exception. I was
thinking about preserving this (because for GNPy, the only element with
no previous or no next nodes are the transceivers, but then Esther's
test topology contains an isolated `fused4` element. If we want to make
this strict, we can do that easily like this:
--- a/gnpy/core/network.py
+++ b/gnpy/core/network.py
@@ -162,10 +162,12 @@ _fiber_fused_types = (elements.Fused, elements.Fiber)
def prev_node_generator(network, node):
"""fused spans interest:
iterate over all predecessors while they are Fused or Fiber type"""
try:
prev_node = next(network.predecessors(node))
except StopIteration:
- return
+ if isinstance(node, elements.Transceiver):
+ return
+ raise NetworkTopologyError(f'Node {node.uid} is not properly connected, please check network topology')
if isinstance(prev_node, _fiber_fused_types) and isinstance(node, _fiber_fused_types):
yield prev_node
yield from prev_node_generator(network, prev_node)
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Co-authored-by: Jan Kundrát <jan.kundrat@telecominfraproject.com>
Change-Id: I41a65e89eef763f82e41e52bc325ed2d488cb601
As pointed out in GitHub issue #390, the normal convention for the sign
of amplifier tilt is to define it with regard to wavelength, i.e.
negative tilt means lower gain for longer wavelengths (lower
frequencies). Currently GNPy uses the opposite convention, which this
patch proposes to change.
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: I8f7829a3b0b0b710f7da013c525630a60b22a2b5
Currently the tilt_target defined by a user is applied over the band of
propagating channels. This means for example that if only two channels
are propagated, the difference in gain between the two channels will be
equal to the tilt_target, independently of how close the two channels
are in frequency. I think it makes more sense to always define the
tilt_target over the full operational bandwidth of the amplifier.
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: I4f29de2edc4d0de239b34e0d8d678d964b6a0af3
As identified in GitHub issue #390, the dgt values (as well as gain and
nf ripple values) in example config json files are listed in order of
increasing wavelength (decreasing frequency) while the code assumes
values listed in the opposite order. This patch reverses the order of
values in affected files so that they are consistent with existing use
in the code.
Also, the f_min value in the Juniper-BoosterHG.json file is updated to
match measurement data so that interpolation is performed correctly.
Change-Id: I97a9d2f9be81380d1658bee5fa1ef4def3e1c537
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Following the change of the corresponding xls file in 60b9256 we should
change also this json file for consistency and to avoid unnecessary
confusion.
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: Iab12002544ad6b8489d8dfa0511fdce762cc1d7a
This is an old pull request rebased and restricted to only raising warning.
The initial work also limited gain, which is finally not a desired behaviour:
an advanced user might want to have this high gain.
the only impact on test is that it raises warnings on almost all amplifiers
on the mesh_topology_exampleV2.xls: indeed all of them are set to low_gain
but without gain specified and the result of autodesign results in higher gains
than supported by this amplifier variety.
This may be confusing for users to see these warnings on an example from gnpy
so I will push a new commit changing the amp types to avoid this.
The alternative would be to push the warnings into the logger, so they
remain invisible, but I think that the example change makes more sense.
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: Idf0c67137b5b466b07ddc7817f53a82f92a21a5b
Patch 503833 (Fix calculation of gain for first Edfa after Roadm)
introduced a bug when it was rebased on top of the per degree power
feature since per_degree_pch_out_db used for propagation was actually
updated based on calculated prev_dp value.
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: I39e8f5945d5035b99c70ef577011bba79bb89a72
The sanity_check function in convert.py removes duplicate links in the
input XLS file. However, it doesn't remove them from the links_by_city
dict. This has two consequences. Firstly, the output json will still
have duplicate connections. Since networkx.DiGraph.add_edge will only
add one edge anyway, this has no major impact. Secondly, sanity_check
will think that a degree-2 ILA node with duplicate links is higher
degree and change it to a ROADM node.
With this patch, duplicate links are removed also from links_by_city.
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: I4be9ab225bc89277aec467a5bd60216b4aa31993
Currently when an Edfa is inserted by auto-design after a Roadm (i.e.
booster) it gets the same city attribute as the Roadm while an Edfa
inserted before a Roadm (preamp) gets the city attribute from the
preceding fiber. This is illogical and confusing. Also both the Edfa
preamp and booster get coordinates different from the Roadm (halfway
between the Roadm and the neighbor node). Since in practice the preamp
and booster are always colocated with the Roadm I think it makes more
sense to give them the same coordinates.
Also change how uid is assigned to an Edfa connected to a Roadm so that
it indicates whether it is a booster or a preamp.
Change-Id: I98718fe1e2914b5628e7cfd23fc28fb5708a8889
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
When loading the equipment file in json_io.py we should raise an error
if an Edfa type_variety specifies a NF type_def that is not
implemented. This should also allow to remove the assert statement in
the _nf method in the Edfa class.
Change-Id: Ida0bb19829c0ee54ecbe3e2f74ea7c22eb24f6a2
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
I think it's better to raise an exception instead like we do for other
errors.
Change-Id: If6dd795bd76471b2534d873772e991d9ae0a4271
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Currently, auto-design does not work for an OMS starting from a Trx
that is not connected through a Roadm if the topology also contains one
or more Roadms. I don't see a good reason not to support this case,
since a Trx not connected through a Roadm can make sense, e.g. for a
degree-1 node. This is a proposal to remove the restriction.
Change-Id: I14686521a838b30249126b9bd403fa26c848875a
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Currently, the RamanFiber class does not implement its own to_json
method but inherits it from the parent Fiber class. This means that
operational parameters (temperature and raman_pumps) are not included
(and therefore not picked up by the network_to_json method in
json_io.py). So if a user saves the topology, e.g. using the
--save-network option, and later uses that saved topology as input,
the result will be wrong.
This patch includes the operational parameters in to_json.
Change-Id: I07c09a4d122858ff412373623d8c0a087a3e11ec
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Currently, if an input topology contains a loop in an OMS, the
set_egress_amplifier method in network.py will go into an infinite
loop. With this patch, such a loop is detected and an error is raised.
Change-Id: Id7cd0cc6d7eaff3af1d9e0309b5c2eb90aeb7454
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
In network.py we are already checking that fibers are properly
connected. Let's check Edfas and other node types as well.
Change-Id: I224b9046729197fef3eb172e9631969a2da13ab5
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
The equipment library has, so far, used completely different namespaces
for `Fiber` and `RamanFiber` definitions. It was possible to define a
"SSMF" fiber whose properties, such as CD or PMD, would differ when used
as a Fiber vs. the RamanFiber object in the networking topology. That is
likely a bug of the equipment library, so this patch makes sure that a
configuration like that cannot be used.
I came across this when working on the YANG support where both fiber
types are defined in a common namespace, and the difference between them
is determined by lack or presence of a sub-container with the Raman
properties.
Change-Id: I8e86bed80eb1794b8abd4e1ecd96398d395f81f2
The patch correction changing the params
from
per_degree_params: { to_node: xx , target_pch_out_db: yy}
to
per_degree_pch_out_db: {xx: yy}
had not been updated on convert.py for reading the excel input.
the commit also fixes the automatic tests with the correct version
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: I17a5cfad18e1b570a7aaa218e932368fa80f2d37
A recent patch introduced the possibility to define an Edfa in the
topology file without specifying its type_variety:
https://review.gerrithub.io/c/Telecominfraproject/oopt-gnpy/+/488967
But with the current implementation, if a user specifies a type_variety
that is missing from the equipment configuration file (e.g. because of
a spelling mistake) this is silently ignored and the type_variety is
instead selected by auto design. I think this is not desired since it
can lead to confusing results. This patch proposes to raise an error
when the specified type_variety is missing while still allowing the user
to not specify type_variety or set type_variety = '' for selection by
auto design.
A recently introduced test actually does exactly this, i.e. it defines
Edfas with type_variety = 'std_high_gain' even though this type variety
does not exist in the equipment config file used by the test. Therefore
this patch also modifies the topology file used by that test.
Change-Id: I7d2a1aa6d633b62d51a99b07e8270cafcbad505f
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
This makes it possible to render at least something on tiny topologies
now that YANG doesn't support city labels.
Change-Id: I1431b557b2eecd34bf24557fdee0da0f2c2c0487
See GitHub issue #368
The out_voa attenuation of the previous Edfa is currently not taken
into account when calculating power target for an Edfa in gain mode.
This makes the calculated gain target (in case of autodesign) for
the following Edfa in the chain incorrect and also impacts automatic
amplifier selection.
Change-Id: Idc473762ccf7b021a0885c7ce20de1abb66eb075
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
See issue reported in #360
In autodesign, currently the calculation of gain for the first Edfa
after a Roadm is incorrect when the reference channel power is
different from 0 dBm. The bug is somewhat hidden by the fact that the
gain is anyway updated during propagation in power mode, taking the
reference channel power into account. But the gain reported by the
to_json function of Edfa elements before propagation will be wrong.
And more seriously, the incorrect gain will impact the Edfa selection
in autodesign.
Change-Id: I004de102832c3a0786435e21e71b0444d8901604
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
This part only targets conversion from an xls input topology file.
In order to define per degree power, the convert function needs to know
the booster final name.
However before this change, the booster name may not be known if there
is no defined amplifier in eqpt sheet at this stage.
In order to solve this ambiguity, the final name are defined in the convert
function provided that the direction is defined in eqpt sheet and
even if the amp type is not defined.
Then the per degrre target power is defined in a new roadm sheet using
the same direction naming as for Eqpt sheet.
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: I8f279aa880aa0a8ceb60e48e43bddb0f6ff06147
- add the per degree info using the EXACT next node uid as identifier
of the degree when a node is a roadm
- add the degree identifier on the propagate and on the call functions
- use the per degree target_pch_out_db defined in json entry for the
target power in network build
- verifies existence of the per degree power target in order to support
partial per degree target power definition
- correct test data files for expected auto design results that now
should include the per degree information, even if it is the same
for all degree.
- in order to enable per degree power definition on direction where
booster is not defined, enable the declaration of edfas in json without
specifying type variety
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: I5004cbb250ca5fd6b6498ac9d4b9c4f28a65efee
In the previous implementation, when a constraint was added on top of
the disjunction, there could be an infinite loop because the test was
not correctly performed on the request_id but on a request class.
Moreover, there the loop was not needed since the first feasible
candidate is selected if it exist.
This patch removes the loop and adds some comments to explain the code.
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: Icdbee9032f28214b3e03cb62d55ea353477d94bf
Since 30234f913c, the code just added the
"system margins" to each transponder's minimal required OSNR. That's
simple and straightforward, but I think that mutating "equipment
library" in response to a "global simulation parameter" is not really
the right way to go.
Make this explicit; always check the resulting OSNR against the
transponder's minimum plus the margin.
I got into this when checking the fixups that are performed within the
JSON loader. I don't think that the JSON loader is an appropriate place
for this.
Change-Id: Ic8f8385cbbb29dc0d769462f17caad47c2b36940
- do not talk about just "Nodes" and "Links" when also checking "Eqpt"
- don't check for non-existing "Nodes" from "Links" when that's already
done elsewhere (and improve that check's error message)
- when checking "Eqpt", verify not just "from_city", but also "to_city"
Change-Id: I2e926049fa5e3c4dcb08cc29f18970a3b3b077d8
The current implementation based on scipy interp1d fails when
predecessor and successor of the fiber to be split have the same
longitude. In this case the new latitudes become NaN. This patch
fixes the bug.
Change-Id: I7c5dc4d410630a6b4b773d36cc192db8271a4346
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Since build_network can raise NetworkTopologyError and
ConfigurationError we should handle these in cli_examples instead of
crashing.
Change-Id: I4b84831c74be7f1c88253c938f3f67b2d204630e
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Other functions used be build_network raises NetworkTopologyError when
a fiber is not properly connected but this handling was missing from
add_connector_loss.
Change-Id: Id08cd4a9bad15f755d364a31ff3d38993d034447
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
