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

feat: Walk on spheres #1022

Merged
merged 45 commits into from May 27, 2022
Merged

feat: Walk on spheres #1022

merged 45 commits into from May 27, 2022

Conversation

keenancrane
Copy link
Collaborator

@keenancrane keenancrane commented May 26, 2022

This PR adds a new example walk-on-spheres. See the README for details.

@keenancrane keenancrane marked this pull request as ready for review May 26, 2022 10:12
@codecov
Copy link

codecov bot commented May 26, 2022

Codecov Report

Merging #1022 (ac212c1) into main (1a00e4c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1022   +/-   ##
=======================================
  Coverage   64.14%   64.14%           
=======================================
  Files          62       62           
  Lines        7945     7945           
  Branches     1802     1802           
=======================================
  Hits         5096     5096           
  Misses       2733     2733           
  Partials      116      116           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a00e4c...ac212c1. Read the comment docs.

@cloudflare-pages
Copy link

cloudflare-pages bot commented May 26, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: ac212c1
Status: ✅  Deploy successful!
Preview URL: https://8b312895.penrose-72l.pages.dev

View logs

@keenancrane keenancrane changed the title Walk on spheres feat: Walk on spheres May 26, 2022
Copy link
Collaborator

@samestep samestep left a comment

Choose a reason for hiding this comment

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

This is incredible, thanks so much for making this @keenancrane! Possibly our most compelling Penrose example yet.

@@ -0,0 +1,22 @@
type Domain -- a region in ℝⁿ
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unicode, nice 😎

Comment on lines +8 to +13
Step x0, x1, x2, x3, x4

x1 := sampleBoundary( x0 )
x2 := sampleBoundary( x1 )
x3 := sampleBoundary( x2 )
x4 := sampleBoundary( x3 )
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is just a matter of aesthetics, but could we do this instead? (same for the other .sub files in this directory)

Suggested change
Step x0, x1, x2, x3, x4
x1 := sampleBoundary( x0 )
x2 := sampleBoundary( x1 )
x3 := sampleBoundary( x2 )
x4 := sampleBoundary( x3 )
Step x0
Step x1 := sampleBoundary( x0 )
Step x2 := sampleBoundary( x1 )
Step x3 := sampleBoundary( x2 )
Step x4 := sampleBoundary( x3 )

@joshsunshine
Copy link
Member

joshsunshine commented May 27, 2022

@samestep I think we should merge this now. I don't think your comment is significant enough to delay.

@keenancrane
Copy link
Collaborator Author

@samestep Yeah, I can see an argument for that coding style also. I think ultimately how this should look is some kind of array syntax:

Step x[5]
for i = 1, .., length(x)
   x[i] = sampleBoundary( x[i-1] )

Of course, that doesn't exist yet! It may also be useful for people to see that declaring all your variables up front is possible as one way to write Substance code. So, I'm gonna leave it for now… (and squash/merge). Thanks!

@keenancrane keenancrane merged commit 5863147 into main May 27, 2022
@samestep samestep deleted the walk-on-spheres branch March 15, 2023 17:25
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.

None yet

4 participants