Skip to content

adding matlab implementation to monte carlo integration #696

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 24, 2020
Merged

adding matlab implementation to monte carlo integration #696

merged 3 commits into from
May 24, 2020

Conversation

foldsters
Copy link
Contributor

No description provided.

PeterMek
PeterMek approved these changes May 24, 2020
View reviewed changes
Copy link

@PeterMek PeterMek left a comment

Choose a reason for hiding this comment

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

It's good code. Runs very quickly, does what it says.

berquist
berquist requested changes May 24, 2020
View reviewed changes
Copy link
Member

@berquist berquist left a comment

Choose a reason for hiding this comment

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

Looks good, just a few comments.

contents/monte_carlo_integration/code/matlab/monte.m Outdated
@@ -0,0 +1,20 @@
monte_carlo(10000000);
Copy link
Member

Choose a reason for hiding this comment

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

This should go at the bottom of the file.

Copy link
Contributor Author

@foldsters foldsters May 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I understand that it looks weird, but matlab instructions (not in a function) in an .m file must come before all function definitions.

Copy link
Member

Choose a reason for hiding this comment

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

Welp, that's what I get for trying to run this in Octave and not knowing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha yeah, matlab is weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed this is resolved then.

contents/monte_carlo_integration/code/matlab/monte.m Outdated
pi_estimate = 4*sum(incircle_array)/n;

fprintf("The pi estimate is: %f\n", pi_estimate);
fprintf("Percent error is: %f%%", 100 * abs(pi_estimate - pi) / pi);
Copy link
Member

Choose a reason for hiding this comment

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

In general, separate the printing from the computation: have monte_carlo compute the estimate for pi, and print the result after the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was following the julia implementation: it also just prints and does not return the value. I can still make the change but I believe what I have is consistent with what's already in the archive.

Copy link
Member

Choose a reason for hiding this comment

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

I just took a look at a few of the other code samples, and most have the printing outside the function except for Julia, so I think the Julia code should be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I will make the change now.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. I'll update Julia today. Thanks for the mention of inconsistency!

@berquist berquist added the Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) label May 24, 2020
foldsters
foldsters commented May 24, 2020
View reviewed changes
contents/monte_carlo_integration/code/matlab/monte.m Outdated
Comment on lines 1 to 4
pi_estimate = monte_carlo(10000000);

fprintf("The pi estimate is: %f\n", pi_estimate);
fprintf("Percent error is: %f%%", 100 * abs(pi_estimate - pi) / pi);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes added!

Copy link
Member

Choose a reason for hiding this comment

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

Just one thing, can you add a newline after Percent error is?

berquist
berquist requested changes May 24, 2020
View reviewed changes
Copy link
Member

@berquist berquist left a comment

Choose a reason for hiding this comment

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

I've downloaded MATLAB through work and, like Peter, confirmed that this works. Just one last request.

contents/monte_carlo_integration/code/matlab/monte.m Outdated
Comment on lines 1 to 4
pi_estimate = monte_carlo(10000000);

fprintf("The pi estimate is: %f\n", pi_estimate);
fprintf("Percent error is: %f%%", 100 * abs(pi_estimate - pi) / pi);
Copy link
Member

Choose a reason for hiding this comment

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

Just one thing, can you add a newline after Percent error is?

@foldsters
Copy link
Contributor Author

I can, but is that really necessary? Any other print that happens after running the file in a program will automatically be run on a new line since it's a new command.

@berquist
Copy link
Member

Yes, but I don't think it looks good:

To get started, type doc.
For product information, visit www.mathworks.com.
 
>> run monte
The pi estimate is: 3.141513
Percent error is: 0.002529%>> run monte
The pi estimate is: 3.140780
Percent error is: 0.025855%>> run monte
The pi estimate is: 3.142591
Percent error is: 0.031772%>>

What if the printed line was really long? Your prompt would be on the right-hand side of the screen.

@foldsters
Copy link
Contributor Author

Oh I see. I was using a jupyter terminal and that didn't happen. I'll get that done now.

@foldsters
Copy link
Contributor Author

Change added

@foldsters
Copy link
Contributor Author

chrome_cZUw66XbEq

For reference here's what it looked like for me using a jupyter console. Thanks for checking on the original ide!

berquist
berquist approved these changes May 24, 2020
View reviewed changes
Copy link
Member

@berquist berquist left a comment

Choose a reason for hiding this comment

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

Nice that you can use Jupyter and aren't stuck with their ancient REPL. Approved!

@berquist berquist merged commit 2333f3b into algorithm-archivists:master May 24, 2020
@leios leios mentioned this pull request May 24, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leios leios leios left review comments

@berquist berquist berquist approved these changes

@PeterMek PeterMek PeterMek approved these changes

Assignees
No one assigned
Labels
Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@foldsters @berquist @PeterMek @leios