Some Homset are not unique parents

[xcaruso]

I guess it is a bug:

sage: k = GF(5) 
sage: H = Hom(k,k)
sage: H2 = Hom(k,k)
sage: H is H2
False

I don't know what is the correct way to fix this problem.

More precisely, in sage.categories.homset (l. 223-227), one can read:

try:
    # Apparently X._Hom_ is supposed to be cached
    return X._Hom_(Y, category)
except (AttributeError, TypeError):
     pass

However, in this particular case, k.Hom is apparently not cached. IMHO, caching should be the job of sage.categories.homset.Hom is all cases, but I might be wrong.

---

Apply

1. attachment: trac_13184_sage_5.9.beta.patch

Depends on #14159

CC: @xcaruso @simon-king-jena @nbruin

Component: categories

Keywords: homset unique parent

Author: Xavier Caruso, Frédéric Chapoton

Reviewer: Travis Scrimshaw

Merged: sage-5.10.beta0

Issue created by migration from https://trac.sagemath.org/ticket/13184

[xcaruso]

comment:1

Attachment: trac_13184_homset_unique_parent.gz

I've attached a small patch fixing the problem.

[jdemeyer]

comment:4

Please fill in your real name as Author.

[xcaruso]

Author: Xavier Caruso

[saraedum]

Attachment: trac_13184.patch.gz

added .patch to the existing patch so the patchbot picks it up

[tscrim]

comment:7

Sage does not run with the patch applied:

/home/travis/sage-5.5.rc0/local/lib/python2.7/site-packages/sage/categories/homset.pyc in Hom(X, Y, category)
    239         # To be investigated
    240         H = X._Hom_(Y,category)
--> 241         _cache[key] = weakref.ref(H)
    242         return H
    243     except (AttributeError, TypeError):

NameError: global name 'weakref' is not defined
Error importing ipy_profile_sage - perhaps you should run %upgrade?
WARNING: Loading of ipy_profile_sage failed.

[xcaruso]

comment:8

I don't understand exactly why you got this error.

Nevertheless, I noticed that ticket #11521 was merged in sage-5.5.beta0 resulting in some important modifications in sage/categories/homset.py. As a consequence, my patch does not apply in versions >= 5.5.beta0. So, I attach a new patch to apply to these recent versions.

[xcaruso]

comment:9

Attachment: trac_13184_sage_5.5.0.beta.patch.gz

[tscrim]

Work Issues: sage fails to run

[tscrim]

comment:10

Same error. I'm running 5.5.rc0. The patchbot agreed with me on the previous version as well (all doctests basically fail), and I suspect it will agree with me on the new patch. Do you have any patches applied in your queue, or if are running something before 5.5.rc0, do you have every patch which modifies homset.py applied before the patch in the queue?

Thanks,

Travis

PS - it's generally not a good idea to have periods in your filenames since they are used for extension types; typically people replace them with underscores.

For patchbot:

Apply only: trac_13184_sage_5.5.0.beta.patch

[fchapoton]

comment:11

here is a rebased and slightly modified patch

for the bot:

apply trac_13184_sage_5.9.beta.patch

[fchapoton]

comment:12

ok, just one little failing doctest, needs to be investigated

[fchapoton]

Changed work issues from sage fails to run to one doctest

[fchapoton]

comment:13

ok, this new patch should pass all tests. Let us wait and see

[fchapoton]

comment:14

for the bot:

apply trac_13184_sage_5.9.beta.patch

[fchapoton]

comment:15

ok, the bot is green. There now remains the question whether or not this is the right thing to do.

Any opinion, somebody ?

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:16

I'm cc-ing Simon to see if he can find a memory leak that I couldn't. However it looks good to me.

[tscrim]

Changed author from Xavier Caruso to Xavier Caruso, Frédéric Chapoton

[simon-king-jena]

comment:17

Is this orthogonal to #14214 and #14279? In any case, I can not review it now, since I am afraid I have to be in holidays.

[tscrim]

comment:18

I believe so, however because of the file rename done in #14214, there will be a dependency one way or the other. I'm cc-ing Nils to let him know about this ticket. Enjoy your holiday Simon.

Nils, what is the review status of #14214 and it's dependency #14159, and perhaps you can find a memory leak that I didn't?

Thanks,

Travis

[nbruin]

comment:19

Replying to @tscrim:

Nils, what is the review status of #14214 and it's dependency #14159, and perhaps you can find a memory leak that I didn't?

If Simon's away you should probably slide this ticket under there. #14214 doesn't fix a bug, it's an enhancement. And I think it needs a little work.

[tscrim]

comment:20

I've made this a dependency of #14214. Thanks Nils.

Frederic, could you change

_cache[key] = KeyedRef(H, _cache.eraser, (signed_id(X),signed_id(Y),signed_id(category)))

to

_cache[key] = H

following #14159 (and remove the commented out line following it)? Thanks.

[tscrim]

Dependencies: #14159

[fchapoton]

comment:21

Attachment: trac_13184_sage_5.9.beta.patch.gz

here is a patch applying above #14159, where I have made the required change

for the bot : apply trac_13184_sage_5.9.beta.patch

[tscrim]

comment:22

Thank you.

[tscrim]

Changed work issues from one doctest to none

[jdemeyer]

Merged: sage-5.10.beta0