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

Feature Request: support 64 bit float #99

Open
tremblap opened this issue Feb 6, 2024 · 17 comments
Open

Feature Request: support 64 bit float #99

tremblap opened this issue Feb 6, 2024 · 17 comments
Labels
enhancement New feature or request

Comments

@tremblap
Copy link
Member

tremblap commented Feb 6, 2024

Is your feature request related to a problem? Please describe the problem.

Pd is moving forward with 64 bit float

Describe the solution you'd like to see.

we should provide both as per the table here:
https://msp.ucsd.edu/Pd_documentation/x4.htm#s1.2.1
we should maybe also change the name of the binaries to acknowledge we distribute FAT ones

Describe alternatives you've considered

users compile themselves, but that will start to be messy soon

Additional context

I presume it is a cmake job

@tremblap tremblap added the enhancement New feature or request label Feb 6, 2024
@tremblap
Copy link
Member Author

tremblap commented Feb 6, 2024

maybe it is a 5 minute job for a magician like @AlexHarker or @weefuzzy - if not I'll tackle it

@AlexHarker
Copy link
Contributor

I suspect this is a cmake job

@weefuzzy
Copy link
Member

weefuzzy commented Feb 6, 2024

I need more information. 64 bit float where? Signal chains, buffers, messages? Is it a matter of rebuilding with a define set or something?

@tremblap
Copy link
Member Author

tremblap commented Feb 6, 2024

Sorry!!! 64-bit float in the signal chain:

https://lists.puredata.info/pipermail/pd-list/2023-12/132768.html

@AlexHarker
Copy link
Contributor

-DPD_FLOATSIZE=64 for a 64 bit float build and then presumably things will likely break. I can probably fix those things, but I am less confident in poking at cmake build systems (particularly to make a new target). CI would also need adapting.

@AlexHarker
Copy link
Contributor

It's not just 64 bit float in the signal chain - it is everywhere.

You probably want to make these changes which avoid explicitly using float and replace with aliases (t_float / t_sample in use already):

https://github.com/AlexHarker/flucoma-pd/tree/feature/64bit-float

Those are totally untested and there are lots of type templates in play so it might be totally broken, but it does compile with the flag set - I'd have to pass to someone else for build system changes.

@weefuzzy
Copy link
Member

weefuzzy commented Feb 7, 2024

Ok cheers. I wouldn't be inclined to touch the cake if this is to be an option, but to pass it in to the build invocation instead. So the main changes would be to CI.

Have buffers also changed? That would be a much bigger set of source changes...

@AlexHarker
Copy link
Contributor

Yes - that would be a possible option - just to pass this as a build option on CI, although you'd need to get a differently named thing out the end (or rename on CI), if you want to build both and be able to install on one computer.

I hadn't checked buffers, but arrays use words, which use t_floats, so yes. I've now updated the branch above with what I think might be the necessary changes there (and checked that it builds). It's not so many places, but again untested, so no guarantee this all propagates nicely and does all the right things - @tremblap could test it though - I just set the compiler flag on my Xcode project (so it would apply to all targets) after running cmake. Alternatively it would probably work to cal the cmake with -DPD_FLOATSIZE=64.

BTW - I always advise touching cake, but only so one can eat it.

CI sounds like a job for @jamesb93 (cough cough) - should he be nice enough to want to do it

@AlexHarker
Copy link
Contributor

Wait @weefuzzy - how do I pass the flag to cmake so it will send it to the right place without altering the cmake?

@AlexHarker
Copy link
Contributor

AlexHarker commented Feb 7, 2024

I've tried export CXXFLAGS="-DPD_FLOATSIZE=64" prior to running the cmake but that doesn't work...

@weefuzzy
Copy link
Member

weefuzzy commented Feb 7, 2024

I've tried export CXXFLAGS="-DPD_FLOATSIZE=64" prior to running the cmake but that doesn't work...

That looks sensible. I'll have a look when I can

@tremblap
Copy link
Member Author

tremblap commented Feb 7, 2024

Thanks all for your thoughts. I have not played with Pd64 yet, but indeed it seems to have 64float everywhere, which is new and frightening/exciting for all our eigen vector views under the hood... so I am terrified to try now.

@weefuzzy before you try to read/think anything, do you want me to try @AlexHarker 's branch and see if I read and write to a buffer via the simplest objects (I think of dataset i/o and bufcompose to start with, literally just copying in and out of fluidland to arrays) or is that not a useful first test?

@AlexHarker
Copy link
Contributor

To build you will need to manually set the flag @tremblap - I can't do that on the branch, because cmake makes the projects ephemeral. I would set it at the project level as in this screenshot

Screenshot 2024-02-07 at 16 31 47

@AlexHarker
Copy link
Contributor

I am cautiously optimistic that it will work. I will bet one cake that it will (at least broadly speaking) work

@AlexHarker
Copy link
Contributor

AlexHarker commented Feb 7, 2024

OK - I should have made sure the whole object set compiled, because the buffer stuff hadn't been hit, and doesn't work, as BufferAdapter assumes 32 bit float. This traces back to possibly various specifically NRT only clients and definitely to the NRT client wrapper. It's not clear to me how many places things would need to change to fix this and there are a few different models.

One is to make BufferAdapter a template (but that would hit a lot of code). Another is to work in some way with preprocessor stuff to change types and have that propagate through. But generally there is quite a high level of pain to go through once a clear design decision has been made.

@weefuzzy
Copy link
Member

weefuzzy commented Feb 7, 2024

Yeah, this was what I feared. I will ponder on the lowest pain way to do this but deffo not a 5 minute job because until now we've been able to assume float buffers everywhere 🥲

@tremblap
Copy link
Member Author

tremblap commented Feb 7, 2024

that was also my hunch (#proudlearner) and it is not a bug but a feature request...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants