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

Standardize intersection in combinatorial_polyhedron/bit_vector_operations.cc #30429

Closed
kliem opened this issue Aug 24, 2020 · 9 comments
Closed

Comments

@kliem
Copy link
Contributor

kliem commented Aug 24, 2020

We standardize the arguments of the intersection to be

inline void intersection(uint64_t *dest uint64_t *A uint64_t *B, size_t face_length)

corresponding well with C[i] = A[i] & B[i] (and with src/sage/data_structures/bitset.pxi).

Previously it was

inline void intersection(uint64_t *A, uint64_t *B, uint64_t *C, size_t face_length)

This is confusing, once you get used to the standard way of doing it (gmp also has mpz_add(mpz_t dest, const mpz_t src1, const mpz_t src2))

CC: @jplab @LaisRast @tscrim

Component: geometry

Author: Jonathan Kliem

Branch/Commit: fe880a4

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/30429

@kliem kliem added this to the sage-9.2 milestone Aug 24, 2020
@kliem

This comment has been minimized.

@kliem

This comment has been minimized.

@kliem
Copy link
Contributor Author

kliem commented Aug 24, 2020

New commits:

fe880a4input of `intersection` in standard order

@kliem
Copy link
Contributor Author

kliem commented Aug 24, 2020

Commit: fe880a4

@kliem
Copy link
Contributor Author

kliem commented Aug 24, 2020

Branch: public/30429

@tscrim
Copy link
Collaborator

tscrim commented Aug 25, 2020

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Aug 25, 2020

comment:6

LGTM.

@kliem
Copy link
Contributor Author

kliem commented Aug 25, 2020

comment:7

Thank you.

@vbraun
Copy link
Member

vbraun commented Aug 30, 2020

Changed branch from public/30429 to fe880a4

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

No branches or pull requests

3 participants