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.

Sign up for GitHub

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

Jump to bottom

bug in add_simplices_from when specifying uid #243

Closed
maximelucas opened this issue Dec 7, 2022 · 3 comments
Closed

bug in add_simplices_from when specifying uid #243

maximelucas opened this issue Dec 7, 2022 · 3 comments

Comments

@maximelucas
Copy link
Collaborator

Running

S = xgi.SimplicialComplex()
S.add_simplices_from([({0, 1, 2}, 0, {})])

does not add (0,1).
That's because we update the uid after adding the subfaces... so it's trying to add (0,1) with id=0 but that id already exists...
@maximelucas
Copy link
Collaborator Author

maximelucas commented Dec 7, 2022 •
edited
Loading

Seems to also come from update_uid_counter()

H = xgi.Hypergraph([({0, 1, 2}, 0, {})])
H._edge_uid
# -> count(0) # wrong

but

H = xgi.Hypergraph([(1,2,3)])
H._edge_uid
# -> count(1) # right

not sure if we created this in the last PR and let it slip or if it was already there.

@maximelucas
Copy link
Collaborator Author

I think it's an off-by-one in that line

https://github.com/ComplexGroupInteractions/xgi/blob/3539c9c10adcdc0441a6ba1617fc1d6872a3981f/xgi/utils/utilities.py#L108

with uid < new_id + 1 it seems to work. @nwlandry can you confirm? But I wonder how we've not see other errors due to this.

maximelucas added a commit that referenced this issue Dec 7, 2022
@maximelucas
fix: off-by-one in update_uid_counter #243
fcbc265
@maximelucas maximelucas mentioned this issue Dec 7, 2022
Merged
maximelucas added a commit that referenced this issue Dec 8, 2022
@maximelucas
Added lost of tests for SimplicialComplex class (#245)
7457018 
* tests: new ones for SC class

* changed from raising errors to warning for deprecated HG methods in SC class

* tests: catch warning

* fix: off-by-one in update_uid_counter #243

* tests: added lots for add_simplices_from

* tests: catch more warnings

* tests: always more in SC class

* style: ran black
@maximelucas
Copy link
Collaborator Author

fixed in #247

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@maximelucas