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

Improved non-recursive version of contFrac in contextFraction.pl. #920

Merged

Conversation

drgrice1
Copy link
Member

This version uses a loop instead of a recursive function. Benchmarking shows it is at least twice as fast. It also should use less memory overhead as it does not store the steps in constructing the continued fraction in arrays. It only keeps what is needed to continue. That is the last two values of the convergent numerator and denominator.

A basic unit test is added to check that this does the right thing. Note that this unit test works with both this branch and the code in develop. Demonstrating that they both generate the same fractions.

I have had this sitting here since March. This is a good change, and I don't know why I didn't put it in sooner.

This version uses a loop instead of a recursive function.  Benchmarking
shows it is at least twice as fast.  It also should use less memory
overhead as it does not store the steps in constructing the continued
fraction in arrays.  It only keeps what is needed to continue.  That is
the last two values of the convergent numerator and denominator.

A basic unit test is added to check that this does the right thing.
Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

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

This looks good. Figured I would try in a problem. Here's a small problem that shows off a little bit:

DOCUMENT();
loadMacros("PGstandard.pl","PGML.pl",'contextFraction.pl');
Context('Fraction')->flags->set(contFracMaxDen => 1000);

$onethird = Fraction(0.33333333); 
$fracpi = Fraction(pi);

BEGIN_PGML
[$onethird]

[$fracpi]
END_PGML
ENDDOCUMENT();

Although the results are identical in both develop and this branch. I didn't benchmark them.

Copy link
Contributor

@Alex-Jordan Alex-Jordan left a comment

Choose a reason for hiding this comment

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

I tested with the golden ratio, which I suspect makes a continued fraction algorithm loop/recurse the most relative to the max denominator, and I had about a 50% improvement in speed with 10,000 consecutive runs.

Something interesting though. When I timed runs individually, the first run takes 3 to 4 times as long as the runs that follow (whether it's thee old or new version). I guess something is loaded that first time that makes the remaining runs faster.

@Alex-Jordan Alex-Jordan merged commit 4600bc0 into openwebwork:develop Aug 24, 2023
2 checks passed
@drgrice1 drgrice1 deleted the context-fraction-contFrac-improved branch August 24, 2023 17:53
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.

3 participants