Work on FlatsMatroid
#38027
Work on FlatsMatroid
#38027
Conversation
|
Documentation preview for this PR (built with commit 9bf7105; changes) is ready! 🎉 |
Add `_closure` and `_is_closed` methods. Create subclass `LatticeOfFlatsMatroid` which results from an input of a _list_ of flats (instead of a dict). This subclass requires more compute upon definition as it computes and stores the lattice of flats from a raw list. On the positive side, given this lattice, some methods become blazingly fast (e.g., `is_valid`, `whitney_numbers`).
There was a problem hiding this comment.
I am not sure about having a full subclass. I think FlatsMatroid should be able to take in the lattice as input and store it (and/or have an optional argument to compute it). If the lattice has been computed (which the user could ask for at some point), then the lattice should be stored and the matroid should chose the code path that take advantage of that. With the proposed change, the flats matroid cannot take advantage of knowing the lattice structure.
It's a bit of an overhaul, but I don't think it will be too difficult. What do you think?
Otherwise, I have some minor comments.
I like it. Quite easy and a good suggestion. Thanks! |
Move functionality of caching and using the lattice of flats (_L) to the main FlatsMatroid class. Suggestion by tscrim.
There was a problem hiding this comment.
Thank you.
The only other thing is I would explicitly set self._L = None and check self._L is None. This is because (in principle) the lattice might be empty, which is planned to eventually be changed so that bool(empty_lattice) is False. This might cause an issue with the corner case of the empty matroid. Plus it makes it a bit more explicit that we are checking if something is initialized.
Done. It is more explicit indeed. I also saved some lines of code from the Please have a look and don't hesitate to keep the corrections coming. :D |
This change makes the code cleaner in multiple places. The PR also includes some docstring edits in `set_system.pyx`. ### ⌛ Dependencies - Depends on sagemath#37930 and sagemath#38027 URL: sagemath#38056 Reported by: gmou3 Reviewer(s): Matthias Köppe
This change makes the code cleaner in multiple places. The PR also includes some docstring edits in `set_system.pyx`. ### ⌛ Dependencies - Depends on sagemath#37930 and sagemath#38027 URL: sagemath#38056 Reported by: gmou3 Reviewer(s): Matthias Köppe
This change makes the code cleaner in multiple places. The PR also includes some docstring edits in `set_system.pyx`. ### ⌛ Dependencies - Depends on sagemath#37930 and sagemath#38027 URL: sagemath#38056 Reported by: gmou3 Reviewer(s): Matthias Köppe
Add class-optimized
_closureand_is_closedmethods.Add input handling of a list of flats, and of a lattice of flats. Note that previously one could only define a
FlatsMatroidfrom a dictionary of flats (indexed by their rank). The definition from a raw list requires more time upon definition as it computes and stores the lattice of flats from the input list. On the positive side, given this lattice, some methods become blazingly fast (e.g.,is_valid,whitney_numbers).The lattice of flats is now cached upon computation.
📝 Checklist