Change tests based on M==0 value for response creation and use
instead the blocking_reason attribute existence
result element should have non null M value if request is not blocked.
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: I67e4222cf9d014201e91d3aefd3624b001264e03
before 'effective-freq-slot' was just ignored, and filled with "null" strings.
this is no longer supported
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: I24d30de91b8b29d37f6ba81220d3cad5aabb6781
For this commit only the first element from the {N, M} list is read
and assigned.
This is better than not reading this value at all.
the commit also updates test_files and test data files with correct
values for the effective_freq_slot attribute
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: I1e60fe833ca1092b40de27c8cbfb13083810414e
Previous set of tests did not correctly check the combinations of
disjunction and route constraint. This new set tests specific cases
with several demands in one synchronization vector with and without
route constraint, and the case where both disjunction constraint and
route can not be met (STRICT and LOOSE cases)
+ minor refactor on test_disjunction
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: Id5a5902e6945185922ce5743ac97d15dbc777af2
Let's use the text mode everywhere because Unicode codepoints is what
matters. The only catch on Windows turned out to be the default file IO
encoding; forcing UTF-8 there fixes all issues in the CI (and it makes
sense because that file was written out in a UTF-8 locale, and the
system which runs the test suite might be set to something else.
This was a rather interesting debugging experience; passing logs over
the web and handling "strange" characters as utf-8 did not help.
Change-Id: I1fdbe3a115458558b27a81f9eab8e58c9605bae7
Bug: https://github.com/Telecominfraproject/oopt-gnpy/issues/358
We have a test which compares the raw output of GNPy against a fixed
expected output. That comparison of course chokes when forward slashes
and backslashes are used, which breaks the test suite on Windows. Let's
try to solve this by always using forward slashes if possible. The way
to go is via pathlib's as_posix(), but that one can possibly return an
absolute path -- which cannot work in a test suite, obviously. So one
can workaround that via calling a Path.relative_to(), but that one
chokes on paths which require at least one "path up" component (`..`).
I posted a patch which use brute force here, but Jonas is right, better
just don't print that output in the test suite in the first place.
Change-Id: I762ddb58a2042120c7b20414152a06a3ed72048d
Bug: https://github.com/Telecominfraproject/oopt-gnpy/issues/358
When `pytest` is run with `-vv`, it shows a diff of multiline strings
and dict just fine. The only drawback is that there's the raw string
with newlines shown as "\n", however, *then* the nice diff pretty
printing kicks in, and the result is:
E Common items:
E {'response-id': '5'}
E Differing items:
E {'path-properties': {'path-metric': [{'accumulative-value': 21.68, 'metric-type': 'SNR-bandwidth'}, {'accumulative-val...EDFA', 'link-tp-id': 'east edfa in Rennes_STA to Stbrieuc', 'node-id': 'east edfa in Rennes_STA to Stbrieuc'}}}, ...]}} != {'path-properties': {'path-metric': [{'accumulative-value': 21.68, 'metric-type': 'SNR-bandwidth'}, {'accumulative-val...EDFA', 'link-tp-id': 'east edfa in Rennes_STA to Stbrieuc', 'node-id': 'east edfa in Rennes_STA to Stbrieuc'}}}, ...]}}
E Full diff:
E {
E 'path-properties': {'path-metric': [{'accumulative-value': 21.68,
E 'metric-type': 'SNR-bandwidth'},
E {'accumulative-value': 28.77,
E 'metric-type': 'SNR-0.1nm'},
E {'accumulative-value': 23.7,
E 'metric-type': 'OSNR-bandwidth'},
E {'accumulative-value': 30.79,
E 'metric-type': 'OSNR-0.1nm'},
E {'accumulative-value': 0.0019952623149688794,
E 'metric-type': 'reference_power'},
E {'accumulative-value': 20000000000.0,
E 'metric-type': 'path_bandwidth'}],
...
... now, it's a bit annoying that there's too much output, but
... that's just for context; the offending lines will be properly
... marked, see --\
... |
... v
...
E {'path-route-object': {'index': 17,
E - 'num-unnum-hop': {'gnpy-node-type': 'transceiver',
E ? ^ ^^^^^^^ - ^ ^^^^^^^^^ -
E + 'num-unnum-hop': {'link-tp-id': 'trx '
E ? ^^ ^ ^^^ ^^
E - 'link-tp-id': 'trx '
E 'Lannion_CAS',
E 'node-id': 'trx '
E 'Lannion_CAS'}}},
E {'path-route-object': {'index': 18,
E 'label-hop': {'M': 6,
E 'N': -274}}},
E {'path-route-object': {'index': 19,
E 'transponder': {'transponder-mode': 'mode '
E '2',
E 'transponder-type': 'vendorA_trx-type1'}}}]},
E 'response-id': '5',
E }
tests/test_parser.py:312: AssertionError
Change-Id: I60b4e3bfa432a720a381bf2c0a9f0288e989dab2
The current JSON data loader preserves (some) integers as integers. When
printed, the value might not contain any decimal points. The YANG patch
series, however, forces floats when floats are expected (while still
allowing None). This makes the output subtly different.
Change-Id: I0e0c013eb3abddb4aeac1ba43bf0d473fed731d4
The term "GSNR" is well established by now. I think it's time we start
using it in our own result outputs instead of alternatives like "total
SNR" or just "SNR".
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Change-Id: I1fc65f6db1e3b2d7cfe974875174132fe5b28d3b
make sure that their loss is not concatenated
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: I63678dd5b72f7f6984101c4367320f3f981cb573
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
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
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
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>
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>
Check that egress power of roadm is equal to target power if input power
is greater than target power else, that it is equal to input power.
Use a simple two hops A-B-C topology for the test where the prev_node
in ROADM B is either an amplifier or a fused. If it is a fused, the target
power can not be met.
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: I3388f060a4f364055d58c8ca7c2b59143f784fa8
- add a test on the json conversion in case of a ROADM sheet
- test the per per_degree_pch_out_db is correctly created with specified
values or default.
- also tests that the convert correctly creates the correct equipment
only if the entry exists in eqpt sheet.
Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
Change-Id: Ic006d9c741404b185d15678953d2801cd17bab97
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
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>
The parameter add_drop_osnr is specified in the Roadm section of
eqpt_config.json and represents noise added by amplifiers within the
add/drop block of ROADMs. Currently this noise is added to the signal
whether the propagation path includes any ROADMs or not, which does not
make sense to me. This patch proposes to only add the add_drop_osnr when
a path actually includes ROADMs.
See GitHub issue #274.
Change-Id: I58961772c049578eff8879dfb2e53265866d12c4
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
(Migrated from GitHub PR #311)
Currently the output of transmission_main_example shows the first
transceiver having ONSR = +inf even when a tx_osnr has been specified,
which is confusing.
This patch proposes to update the SNR of the first transceiver with
tx_osnr.
Change-Id: Idab7c92c2f5a12cc92ce5c1c551e5710f30e6a02
Signed-off-by: Jonas Mårtensson <jonas.martensson@ri.se>
Esther mentioned that it is useful for her to be able to convert from
XLS files to JSON files. Let's add a full blown script for this.
I've also taken the liberty to refactor the code a bit so that there's
no default value, and to modernize everything with pathlib a little bit.
Change-Id: I80e50fc1280003910242ce1ff9fc9ae66e6d275b
I would like to create a package for distribution to PIP, and this seems
like the path of least resistance.
This is, apparently, the way for shippign arbitrary data with Python
[1]. I've at least tried to make it user-firendly via adding a simple
utility which just prints out whatever that data path is.
[1] https://python-packaging.readthedocs.io/en/latest/non-code-files.html
Change-Id: I220ecad84b1d57d01e3f98f15befc700bd97c0b8
It's a bad habit to write files into the source code repository. It will
also become impossible if gnpy is installed into a systemwide, possible
read-only location.
The old behavior can be reactivated by using an extra option to tell
GNPy where to put the generated file.
Change-Id: I9ad43890ca5886f516885de5938a4778870a06c5
As I'm moving the top-level directory `examples/` to another place, I
wanted to clear the source of any mentions of examples which are not
actually valid paths.
Change-Id: If6cce20feacfbbb79549e865d06aa00fd2dcd08d
Since Ic4a124a5cbe2bd24c56e4565d27d313fe4da703f, there was no automated
test which would check if the generated examples *really* work. When I
was playing with this, I managed to break it at least once (especially
when working on overriding sys.args, i.e.,
I53833a5513abae0abd57065a49c0f357890e0820).
This now requires an equivalent of `pip install` before the tests can be
run.
Change-Id: I595a3efe29b3ee13800c5cb71f28a5f370988617
I would like to avoid that extra fork to a child Python interpreter (it
looks like something that can be easily avoided). It's something that's
possible now that the code ships just some trivial wrappers (which are,
in turn, needed for setuptools' `console_scripts`).
This cannot use the `capsysbinary` fixture for wrapping of stdout/stderr
due to something in pytest which already got fixed, but has not been
released yet (May 2020). Let's use `capfdbinary` which works fine.
Change-Id: Ic4a124a5cbe2bd24c56e4565d27d313fe4da703f
See-also: https://github.com/pytest-dev/pytest/pull/6926
