Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repeatedly calling core.network.Network.interregional_betweenness gives wrong results #124

Closed
jkroenke opened this issue Apr 30, 2019 · 2 comments
Assignees
Labels
Milestone

Comments

@jkroenke
Copy link
Contributor

When a network is initialized and interregional_betweenness is called

net = Network.SmallTestNetwork()
net.interregional_betweenness(sources=[2], targets=[3, 5])

the result

array([1., 1., 0., 0., 1., 0.])

is correct for the first calculation. However, if interregional_betweenness is called a second time with different source and/or target nodes

net.interregional_betweenness(sources=range(0, 6), targets=range(0, 6))

the result is still the same. For a newly initialized network it is different:

array([9., 3., 0., 2., 6., 0.])
@jkroenke jkroenke added the bug label Apr 30, 2019
@jkroenke jkroenke self-assigned this Apr 30, 2019
@jkroenke
Copy link
Contributor Author

The problem occurs in core.network.Network.nsi_interregional_betweenness, too. As both call nsi_betweenness, I suspect the problem comes from there.

@ntfrgl
Copy link
Member

ntfrgl commented Apr 30, 2019

@cached_const is wrong here. It should be either deleted or replaced with @cached_var. In the latter case, the decorator would need to be patched to handle sources=..., targets=... instead of key=....

Or, probably rather use: https://joblib.readthedocs.io/en/latest/generated/joblib.Memory.html

@ntfrgl ntfrgl added this to the Release 0.7 milestone Dec 1, 2023
ntfrgl added a commit that referenced this issue Feb 21, 2024
Improve the approach in reverted commits fe23f23, 0166cd2:

- introduce `@Cached.method()` based on `@functools.lru_cache()`
- track mutable dependencies for cache invalidation
- fix cache clearing helper
- add behavioural spec in `tests/test_core/test_cache.py`

Adjustments in existing classes in this commit:

- define `__cache_state__()` for all subclasses of `Cached`
  (might require updates when adding further methods to the cache)
- define `@Cached.method(attrs=(...))` where appropriate
- remove obsolete class-specific caching mechanisms
- factor out cached worker: `Network.nsi_betweenness()`
- define helper: `Network.find_link_attribute()`
- define helper: `ClimateNetwork._weighted_metric()`

Adding this behaviour to classes without caching should be straightforward.
Classes that remain to be adjusted (still have a `clear_cache()` method
or subclass `Cached` without conforming to its protocoll):

- `ClimateNetwork` and its subclasses:
    The recursive interaction between `__init__()`, `_similarity_measure`
    and `_regenerate_network()` across the class hierarchy is very
    stateful and does not fit into the regular OO dependency pattern
    assumed by `Cached`. A redesign of this logic would be advisable,
    but is left for future work.
- `RecurrenceNetwork` and its subclasses, and `Surrogate`:
    Left as an exercise for the reader.
fkuehlein added a commit that referenced this issue Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants