Skip to content

Create package-global ACSVSettings class #21

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

Merged
merged 14 commits into from
Mar 13, 2025
Merged

Conversation

behackl
Copy link
Collaborator

@behackl behackl commented Mar 10, 2025

This implements a new mechanism for OutputFormat which can be used to set a global default.

If you like this sort of interface, we should also (a) expose OutputFormat to the top-level import (so from sage_acsv import OutputFormat works) and (b) perhaps think about renaming OutputFormat to signify that this is something belonging to our package -- perhaps ACSVFormat or so ...

@behackl behackl added the enhancement New feature or request label Mar 10, 2025
@behackl
Copy link
Collaborator Author

behackl commented Mar 10, 2025

CC @smelczer @turnip314, this is my suggestion for a global output configuration. In short, currently:

sage: from sage_acsv.helpers import OutputFormat
sage: OutputFormat.get_default()
<OutputFormat.SYMBOLIC: 'symbolic'>
sage: OutputFormat.set_default('asymptotic')
sage: OutputFormat.get_default()
<OutputFormat.ASYMPTOTIC: 'asymptotic'>

... and diagonal_asy etc. respect whatever the default is set to whenever output_format=None (the default) is passed.

Comments are welcome!

(This also fixes a bunch of the doctests; one of them appears to be still broken though, might be a bug on the non-smooth branch.)

@smelczer
Copy link
Member

That looks good to me (and I like ACSVFormat).

I suppose we can do the same thing for M2 and msolve. There are probably two ways to do it: have a boolean flag for both, or have something like ACSVGrobnerBackend which the user can set to M2 or msolve and otherwise defaults to the Sage default (I guess singular?). The benefit of the first way is that we can be flexible about where we use each package, but that means we need to know which is better in which circumstance, while the benefit of the second is the user knows precisely which backend will be used for all computations. I guess I prefer the second option (use msolve or M2 globally) although this means we will need to adapt some code that currently only uses the Sage default or M2 (I'm thinking in particular of code in Whitney).

@turnip314
Copy link
Collaborator

Looks good to me too.

For some reason I can't see the doctest results. The little green/red icon beside each commit stopped appearing a while ago...

One of the doctests that might be failing is for the Whitney Stratification, since I notice you didn't make any modifications to that file.

@behackl
Copy link
Collaborator Author

behackl commented Mar 12, 2025

I've reworked my original proposal to create a single source of truth (the ACSVSettings class) that should hold all configurable settings.

If you are still okay with this, I'd also like to move the logger verbosity (ACSVSettings.(get|set)_verbosity) to this.

It would also be a sort of natural place where the Macaulay2 interface could be configured (ACSVSettings.Macaulay2). Personally, I'd be fine with removing the m2 keyword arguments from the methods in whitney.py and asymptotics.py then -- but perhaps you'd like to preserve the option for diagonal_asy; I'm not sure what the cleanest approach here is.

@behackl behackl changed the base branch from non-smooth to main March 12, 2025 13:54
@behackl behackl changed the title Allow setting default output format globally Create package-global ACSVSettings class Mar 12, 2025
@turnip314
Copy link
Collaborator

I think it would be better to remove the m2 keyword and have the user choose their backend preferences through global configuration settings. So the user can do something like ACSVSettings.configure_M2(path=path_to_M2_installation) and ACSVSettings.set_preferred_backend(ACSVSettings.Backends.M2). When diagonal_asy or any other function is called, it will try to use the user's preferred option whenever it's available.

@smelczer
Copy link
Member

I agree with @turnip314

@behackl
Copy link
Collaborator Author

behackl commented Mar 12, 2025

Great, I like this as well!

One thing that I am not sure about yet: currently there are two choices for the Gröbner basis computation in kronecker.py (default vs. msolve), and again two choices for computing the primary decomposition of ideals in whitney.py.

Should these be two separate config options (ACSVSettings.GroebnerBackend and ACSVSettings.DecompositionBackend), or just one option (which, as far as I can tell, would require additional work to implement everything with either backend)?

@smelczer
Copy link
Member

Eventually we will probably want one backend, but for now we can just do what's easiest.

@behackl behackl merged commit 652fd80 into main Mar 13, 2025
5 checks passed
@behackl behackl deleted the default-output-setting branch March 13, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants