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

Add list of cached functions #3599

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aaruni96
Copy link
Member

Addresses #3450

@@ -0,0 +1,323 @@
# Functions Which Cache Things

Many functions take in a keyword argument `cached::Bool`, which then cache or do not cache the results of computations to make things faster. This can sometimes lead to strange behaviour. So we document all the functions which do this, and at a future date, maybe do something with this list.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"cache or do not cache the results of computations to make things faster"
I don't think this is what is happening here.

To my understanding there are two reasons for caching in OSCAR:

  • Functions which do complex computations and cache the result (like groebner_basis or maximal_order), so it does not need to be computed again. These functions usually do not have (maybe never have) a cached keyword.
  • Constructors which add the constructed parent object to a global dictionary (= cache), so that if people construct the same polynomial ring or whatever twice, they get the identical ring and everything will just work. This is what the cached keyword is for. (So it is not about speed but convenience.)

Please correct me, if I am wrong here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be completely right. For now, I've removed that, and just added a "FIXME". Someone more qualified than me will have to figure out how to best phrase that paragraph.

@lgoettgens
Copy link
Member

Can you adapt it so that the functions get put into code blocks (I.e. surrounded by backticks)? That should nice up the printing

@joschmitt
Copy link
Member

I added some description of the different ways 'caching' occurs in OSCAR (at least the ones I'm aware of), I hope you don't mind @aaruni96.
This is just reflecting my understanding of things; somebody should check this for correctness and also decide how much we want to document these things at all.

true
```
This is mainly intended for interactive use where this behaviour may be convenient.
However, the parent objects constructed in this way cannot be deleted (that is, garbage collected) as they are stored in the dictionary.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is wrong, at least this is what I understand from the WeakValueDict docstring in AbstractAlgebra

WeakValueDict() constructs a hash table where the values are weak
references to objects which may be garbage collected even when
used as values in a hash table.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how I understood it from the OSCAR book.
But yes, apparently garbage collection works:

julia> Qx, x = QQ[:x]
(Univariate polynomial ring in x over QQ, x)

julia> Oscar.Nemo.FmpqPolyID
AbstractAlgebra.WeakValueDict{Symbol, QQPolyRing}(...):
  :x => Univariate polynomial ring in x over QQ

julia> Qx = 0
0

julia> x = 0
0

julia> GC.gc()

julia> Oscar.Nemo.FmpqPolyID
AbstractAlgebra.WeakValueDict{Symbol, QQPolyRing}()

Conclusion: I'm not qualified either to write this documentation.

Comment on lines +249 to +250
> - `similar(f::PolyRingElem, R::fqPolyRepField, s::Symbol=var(parent(f)`
> - `similar(f::PolyRingElem, R::FqPolyRepField, s::Symbol=var(parent(f)`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaruni96 your magical pipe seems to cut off some function signatures like here. (Just saying. As long as we don't know what we do with this list, there is probably no point in debugging this.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, it cuts off at the first closing parenthesis. Its probably worth fixing, as it might be confusing later as to why something without cached seems to appear in the list.

I'll change the sed pattern.

@fingolfin
Copy link
Member

So what should we do with this list? Put it somewhere? But it will get outdated quickly. Or put a program to generate the list somewhere?

Or add another section to the "General" section of the OSCAR manual? Maybe in https://docs.oscar-system.org/dev/General/faq/ or a new section "Things to be aware of when using OSCAR" (which then could also mention we have our own matrix and integer types, which is right now in the FAQ)

@micjoswig since you asked for this, would be good to hear from you what you'd prefer

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

Successfully merging this pull request may close these issues.

4 participants