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

tnet.T incorrect when tnet is sparse and starttime!=0 #61

Open
maximelucas opened this issue Mar 9, 2020 · 6 comments
Open

tnet.T incorrect when tnet is sparse and starttime!=0 #61

maximelucas opened this issue Mar 9, 2020 · 6 comments

Comments

@maximelucas
Copy link
Contributor

maximelucas commented Mar 9, 2020

Running

import teneto as tnt
import pandas as pd

netin = {'i': [0,0,1,1], 'j': [1,2,2,2], 't': [100,101,102,103]}
df = pd.DataFrame(data=netin)
tnet = tnt.TemporalNetwork(from_df=df)

print(tnet.T)

displays 104, even when specifying starttime=100 at the creation of the network. From the documentation, T is

number of time-points in network

so that it should be 4.

@maximelucas
Copy link
Contributor Author

maximelucas commented Mar 9, 2020

I believe the problem comes from the _calc_netshape() function in the teneto/classes/network.py file. When the network is not sparse (branch else of the conditional statement) T is assigned the value
T = self.network['t'].max()+1
which only takes the end time into account, not the start time. Solve by replacing with
T = len(set(self.network['t']))
which yields 4. It also deals with missing time points in the following way: if in netin dictionary there is
't': [100,100,102,103]
with the 101 time point "missing", the new line yields 3.

@wiheto
Copy link
Owner

wiheto commented Apr 22, 2020

Sorry for taking over a month top get back to you on this.

Nice catch. And the fix would work quite well. But I think the following would be a slightly better solution:

T = self.network['t'].max() - self.network['t'].min() + 1

Because, so in the case if there is [100, 100, 102, 102, 103], I think it would be better to display T=4, instead of T=3. Mainly because T would be 4 if this was converted into a numpy array.

Feel free to submit a PR if you want to. Otherwise I'll add this in the next version I release.

@maximelucas
Copy link
Contributor Author

maximelucas commented May 12, 2021

It took me way longer to reply, oops.
Thanks, I saw you replaced the line in the file.

One more question, though: this does not work for decimal times, e.g. [100, 100.2, 100.3], which would yield T=1.3.
Actually I checked and teneto returns T=1, which does not seem very natural in any case.
Are decimal times not allowed in teneto so far?
I've not seen them forbidden or checked for, but maybe functions assume integers.

Related: as far as I understand it, teneto assumes a constant time step. So if I data that corresponds to times non uniformly distributed, there is no natural way to deal with them in teneto, si that right? For example times like [10, 11, 20, 30, 100], teneto would enforce a timestep of 1 and add lots of unexisting timepoints. If this is the case I can raise an issue for an enhancement for example, but I wanted to make sure.

@wiheto
Copy link
Owner

wiheto commented May 12, 2021

Are decimal times not allowed in teneto so far?

I've never got round to making non-discrete temporal networks. some functions may break if you include decimals in the time stamps (i have not tested this though)

timelabels is a possible option for you - this will just be a list (e.g. [100, 100.1, 100.2, 100.3]) that then map to the timepoints [0, 1, 2, 3] in figure functions.

In some functions there is the option of adding the sampling frequency. But that has not been included in TemporalNetwork class yet as other functions got prioritized.

@wiheto
Copy link
Owner

wiheto commented May 12, 2021

Related: as far as I understand it, teneto assumes a constant time step. So if I data that corresponds to times non uniformly distributed, there is no natural way to deal with them in teneto, si that right? For example times like [10, 11, 20, 30, 100], teneto would enforce a timestep of 1 and add lots of unexisting timepoints. If this is the case I can raise an issue for an enhancement for example, but I wanted to make sure.

It's generally designed to use uniform timepoints in many functions. But it is fine if the time-stamps are non-uniform as well. If something breaks, let me know. However, it will be better than to ensure that the network is sparse (forcesparse) otherwise the numpy arrays will be unnecessarily large.

I think all this functionality would be wonderful! It's all about time to implement it all.

@maximelucas
Copy link
Contributor Author

Thanks for the clarifications!
Of course, it all takes time to build!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants