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

bijectionist does avoidable work #39307

Open
2 tasks done
mantepse opened this issue Jan 9, 2025 · 2 comments · May be fixed by #39313
Open
2 tasks done

bijectionist does avoidable work #39307

mantepse opened this issue Jan 9, 2025 · 2 comments · May be fixed by #39313

Comments

@mantepse
Copy link
Collaborator

mantepse commented Jan 9, 2025

Steps To Reproduce

Suppose we are calling

sage: Bijectionist(A, B)

with large sets A and B, eg., with 20000 elements each, as in the doctest.

Expected Behavior

This should return almost immediately, since no work needs to be done.

Actual Behavior

Instead, it takes a long time. This is because __init__ finally calls self.set_constant_blocks([]), which in turn calls self._compute_possible_block_values(), which will then compute the intersection of the two element sets

([self._restrictions_possible_values[a] for a in block] + [self._statistics_possible_values[a] for a in block])

for each block of self._P. Here self._P is a DisjointSet of singletons - one block for each element of self._A. In fact, this will always be the value of self._P, whenever we are looking for a bijection (i.e., self._tau is the identity map).

However, initially, these two sets are all of self._A.

So:

  • check whether we can avoid calling self._compute_possible_block_values()
  • check whether we can optimize self._compute_possible_block_values() for the case of very few non-singleton blocks.

Additional Information

No response

Environment

irrelevant.

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@mantepse
Copy link
Collaborator Author

mantepse commented Jan 10, 2025

While looking into this I also realised that _preprocess_intertwining_relations has no real example - both examples provided actually do not modify _P.

Also,

        .. TODO::

            it is not clear whether this method makes sense

is a little discouraging.

@mantepse
Copy link
Collaborator Author

mantepse commented Jan 10, 2025

Note to myself: I need to track users of _possible_block_values, because this is what _compute_possible_block_values sets. These are

  • _forced_constant_blocks
  • possible_values
  • _find_counterexample
  • and all of _Bijectionist_MILP

@mantepse mantepse linked a pull request Jan 10, 2025 that will close this issue
vbraun pushed a commit to vbraun/sage that referenced this issue Jan 19, 2025
…ing_relations

    
This is related to sagemath#39307.  I need to understand precisely how
manipulation of `_P`, `_compute_possible_block_values`, etc. interact,
and this is a tiny step in this direction.
    
URL: sagemath#39310
Reported by: Martin Rubey
Reviewer(s): David Coudert, Martin Rubey
vbraun pushed a commit to vbraun/sage that referenced this issue Jan 19, 2025
    
Fix sagemath#39307

Since the package is currently probably a bit under-used, this should be
checked thoroughly.
    
URL: sagemath#39313
Reported by: Martin Rubey
Reviewer(s): David Coudert, Martin Rubey
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant