[louisgregg]

Description

A single node in networkinfo_staging.gml graph doesn't contain the country_code key in its dictionary - node 284. Contents of the node dict:

{'ip_address': '185.243.20.232', 'bandwidth_down': '35408 Kibit', 'bandwidth_up': '21936 Kibit'}  

In the rare case that this particular node is then used to construct the network graph for the shadow simulation, I encounter an error in a geographical analysis I'm running on subsequent simulation output. This PR creates a optional flag for the stage subcommand which removes a node from this graph if it doesn't contain a country_code key.

Flag Usage

tornettools stage --help
# ...
#   --remove_nodes_without_country_code
#                        Include only those nodes in the networkinfo_staging.gml network graph that specify a country_code (default: False)

Testing

I smoke tested this on my local simulation / analysis pipeline but I haven't created any unit tests or integration tests, etc.

[robgjansen]

I wonder if the pruning should be done upstream, on the code that generates the input that is passed to the staging command?

If we merge this, I'd ask for some minor changes first.

[louisgregg]

I wonder if the pruning should be done upstream, on the code that generates the input that is passed to the staging command?

By default, networkinfo_staging.gml is a nodes-only copy of tmodel-ccs2018.github.io/data/shadow/network/atlas_v201801.shadow_v2.gml.xz . Would you prefer to see this country_code check implemented in the tmodel-ccs2018.github.io repo itself? That's a data-only repo so I don't know where this logic should go there.

tornettools stage is the first step in costructing/running a tornettools simulation so to me, it seemed like an appropriate point to remove the nodes. Let me know if I've misunderstood your suggestion, thanks.

If we merge this, I'd ask for some minor changes first.

Done.

[robgjansen]

OK, this looks good.

Note, I think the upstream scripts are in our atlas repo. And I think the script that outputs the final graph file is here.

I have not inspected the above script to see how it would need to change so that it does not output empty country codes, but I think in any case it makes sense to have a defensive check here so I'm approving this PR.

Thank you for your contribution!

(I will inspect the CI issue before merging.)