PlummersSoftwareLLC / Primes

Prime Number Projects in C#/C++/Python
https://plummerssoftwarellc.github.io/PrimeView/
2.43k stars 574 forks source link

Primes in Pyret #949

Closed rzuckerm closed 7 months ago

rzuckerm commented 7 months ago

Description

Implementation of Primes in Pyret.

Contributing requirements

rbergen commented 7 months ago

@rzuckerm Thank you for submitting this. I always enjoy adding a "new" language to the collection.

After taking a first look at your solution and the Pyret language, I do think some changes will have to be made to be able to mark the solution as "faithful". As per the contributing guidelines, a "faithful" solution must

[use] a class to encapsulate the sieve, or a (closest) equivalent feature in your language if a class construct is not available. This class must contain the full state of the sieve. Each iteration should re-create a new instance of this class from scratch.

In my understanding of Pyret, it is possible to implement a class-like construct using data declarations, which includes the possibility to incorporate methods that operate on the contents of the actual data type.
I'm therefore of the opinion that a "faithful" Pyret solution should be based on the data declaration construct.

Would it be possible for you to restructure the solution accordingly?

rzuckerm commented 7 months ago

@rbergen I'll try. The pyret documentation is not great. I looked over the data declaration and couldn't quite understand how to use it.

rzuckerm commented 7 months ago

@rbergen I think I have what you requested. The closest thing to a class in Pyret is a data declaration.

rbergen commented 7 months ago

@rzuckerm Thank you for taking the time and trouble to adapt the solution to use data declarations. I'll test and review a little later today.

rbergen commented 7 months ago

@rzuckerm I may not have explained sufficiently clearly that the use of the arch-* flag file really is a last-resort measure that should only be used if creating a multi-architecture Docker image is not possible.

I've just built a combined linux/amd64 and linux/arm64 pyret image off of your pyret-docker-image repo myself, and pushed it to primeimages/pyret:0.0.27-2 (available on Docker Hub here: https://hub.docker.com/r/primeimages/pyret). I then changed the FROM line in the Dockerfile to pull that image instead of yours, and tested the solution on both architectures. That worked, which means it is possible to build and run your Pyret image and hence this solution on arm64 as well.

As I see it we can now do either of two things:

Obviously, in both cases the arch-amd64 flag file should once again be removed.

I'll leave the decision on which approach to take to you.

rzuckerm commented 7 months ago

@rbergen I changed my image to also support linux/arm64

rbergen commented 7 months ago

Great, thanks! Your ARM64 image also works on my ARM64 device. I'll delete the primeimages Pyret Docker image, as that is now redundant. This being a new solution in a new language, I'll request a second review from @marghidanu and give him a few days to give it, if he wants to.