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

Addition on CombinatorialFreeModule directly on dictionaries #9651

Closed
stumpc5 opened this issue Jul 31, 2010 · 13 comments
Closed

Addition on CombinatorialFreeModule directly on dictionaries #9651

stumpc5 opened this issue Jul 31, 2010 · 13 comments

Comments

@stumpc5
Copy link
Contributor

stumpc5 commented Jul 31, 2010

At the moment, adding (and taking negative, substracting,...) for CombinatorialFreeModule is not done in a very efficient way.

This patch

======================================

prerequisite:
trac_9648_modulemorphism_codomain_extension.2.patch

apply that patch and
trac_9651_CombinatorialFreeModule_Addition-cs.4.patch,
which supercedes the previously posted patches on this page.

Component: combinatorics

Keywords: addition of dictionaries, CombinatorialFreeModule

Author: Christian Stump

Reviewer: Daniel Bump

Merged: sage-4.6.1.alpha0

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

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jul 31, 2010

comment:1

Attachment: trac_9651_CombinatorialFreeModule_Addition.patch.gz

The patch still needs some doctesting (I have not yet done much, but will do more these days) - in particular, I modified CombinatorialFreeModule._apply_module_morphism and .apply_module_endomorphism. The first method is used in the code for symmetric functions: powersum.py, sfa.py and macdonald.py. Would be nice if someone could check if everything there still works well.

Thx, Christian

@dwbump
Copy link
Mannequin

dwbump mannequin commented Sep 6, 2010

comment:2

The patch doesn't apply cleanly to sage-4.5.3.rc0. Applying #9648 first doesn't help.

@stumpc5
Copy link
Contributor Author

stumpc5 commented Sep 19, 2010

@dwbump
Copy link
Mannequin

dwbump mannequin commented Sep 22, 2010

comment:3

The revised patch trac_9651_CombinatorialFreeModule_Addition-cs.patch
applies cleanly to sage 4.6.alpha1 and passes all tests.

But I haven't been able to confirm that it is a speedup. The results
of some testing are here:

http://groups.google.com/group/sage-combinat-devel/msg/4869cc885ca42bc1

@stumpc5
Copy link
Contributor Author

stumpc5 commented Oct 16, 2010

@stumpc5
Copy link
Contributor Author

stumpc5 commented Oct 20, 2010

includes referees suggestions

@stumpc5
Copy link
Contributor Author

stumpc5 commented Oct 20, 2010

Attachment: trac_9651_CombinatorialFreeModule_Addition-cs.3.patch.gz

Attachment: trac_9651_CombinatorialFreeModule_Addition-cs.4.patch.gz

includes more referees suggestions

@dwbump
Copy link
Mannequin

dwbump mannequin commented Oct 20, 2010

Reviewer: Daniel Bump

@dwbump

This comment has been minimized.

@dwbump
Copy link
Mannequin

dwbump mannequin commented Oct 20, 2010

comment:4

Positive review!

There is a thread about this patch in sage-combinat-devel
resulting in the latest version.

I tested this with sage-4.6.alpha3. After applying

trac_9648_modulemorphism_codomain_extension.2.patch 
trac_9651_CombinatorialFreeModule_Addition-cs.4.patch

all tests pass. I also confirmed that the test is a speedup.

@jdemeyer jdemeyer added this to the sage-4.6.1 milestone Oct 23, 2010
@jdemeyer
Copy link

comment:6

Please remove * * * from the commit message

@stumpc5
Copy link
Contributor Author

stumpc5 commented Oct 23, 2010

comment:7

Attachment: trac_9651_CombinatorialFreeModule_Addition-cs.5.patch.gz

Replying to @jdemeyer:

Please remove * * * from the commit message

done!

@jdemeyer
Copy link

jdemeyer commented Nov 1, 2010

Merged: sage-4.6.1.alpha0

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

No branches or pull requests

2 participants