I believe a previous commit, 3ac08f5, changed the behaviour of span
loss calculation in an unintended way, since now it adds the loss of
consecutive fiber elements even when there is no fused element in
between. This means for example that no padding is added when the loss
of consecutive fibers is higher than the padding specified in the
equipment file even though inline amplifiers will be added between the
fibers in a later step. This patch changes the conditions in the next_
and prev_node_generator so that they stop when two consecutive fibers
are found.
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: I42c9188c789a98a9b3d7e51d5aae15774d40dde7
Fix GitHub issue #217
Currently, if a user specifies an ILA node in an xls file, including
city name and coordinates, but does not specify type of amplifier,
etc., in the Eqpt sheet, the ILA node is not preserved when converting
to json. This patch proposes to include all ILA nodes to prevent loss
of information.
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: Id169348cce185e4d33d5b80068270b36043e3353
Currently, calculated new fiber coordinates, after splitting a fiber by
auto-design, are evenly distibuted. Since coordinates for added inline
edfas are the midpoint between neighboring nodes, this makes the edfas
"look" non-equidistant even if all spans have the same length. I think
it would make more sense to have the fiber coordinates represent the
midpoint of the fiber. That way, the edfa positions will look more
"natural". Here is an attempt to illustrate the difference for a link
with three fiber spans:
Before this patch:
r-----f--e--f--e--f-----r
After this patch:
r---f---e---f---e---f---r
r = roadm
e = edfa
f = fiber
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: I6eafe3fcd4c718b0b995a046dbff0fd04bdc42d7
This could be (potentially) annoying to those users who rely on the
default equipment library. However, it brings at least some order into
the current state -- which was rather disorganized.
Suggested-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: Ifc3ec5f9e0e2526b8621d905160fc82af6a469f2
The NF calculated by the preamp model is compliant with the MW-MW noise
mask in the OpenROADM MSA spec. The booster is noise-free, which is
modeled by setting the NF to zero (-inf in dB units). This is obviously
unphysical but it is the simplest way to model the total noise
contribution from a ROADM, including preamp and booster, that is
compliant with the the OpenROADM MSA.
This also introduces two new EDFA type varieties,
"openroadm_mw_mw_preamp" and "openroadm_mw_mw_booster" in the equipment
library. I would prefer to also change the names of the existing
"openroadm" type_def and "standard"/"low_noise" type_variety,
representing an OpenROADM inline-amplifier, for better consistency but
this probably needs to be discussed first.
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: I7344ff53623f166649efb389c95c04ff92718e35
Signed-off-by: Jan Kundrát <jan.kundrat@telecominfraproject.com>
Co-authored-by: Jan Kundrát <jan.kundrat@telecominfraproject.com>
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
This allows users to limit the choice of type_variety by auto-design
for an EDFA node by setting a "variety_list" attribute in the input
topology json file. One use-case is switchable gain EDFAs where the
two gain ranges can be modeled by two separate type varieties in the
equipment library. A user may know that such an EDFA will be used in
a node but not which gain range is optimal. The choice of gain range
can then be left to auto-design while not allowing any other
type_variety by specifying the node e.g. like this in the topology:
{
"uid": "Edfa1",
"type": "Edfa",
"variety_list": ["foo_gain_range_1", "foo_gain_range_2"],
...
}
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: Ia69ef78f885e3a61310530b6b80da64e43758211
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>
