Skip to content

Conversation

fchapoton
Copy link
Contributor

extracted from #37601

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.

Copy link

github-actions bot commented Aug 29, 2025

Documentation preview for this PR (built with commit fd7eab7; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@mantepse
Copy link
Contributor

Looks good, I tested against oeis and findstat.

I just wanted to suggest that we change the signature to allow only testing semimodularity of a single element, but then I saw is_modular, so the interface is good, too.

Oh, we already have is_left_modular_element, with a slightly different implementation. It's a bit strange, it seems more than twice as fast on lattices with 9 elements:

sage: from sage.databases.findstat import _finite_lattices
sage: l = list(_finite_lattices(9))
sage: %timeit a = [L.is_left_modular([x]) for L in l for x in L]
5.25 s ± 146 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
sage: %timeit b = [L.is_left_modular_element(x) for L in l for x in L]
1.89 s ± 39.9 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

@fchapoton
Copy link
Contributor Author

The method is_left_modular_element was only using the cover relations. Now, I do the same in the new method.

@mantepse
Copy link
Contributor

Thank you!

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

Successfully merging this pull request may close these issues.

2 participants