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

Use enum for precision #167

Open
davebayer opened this issue Mar 13, 2024 · 2 comments
Open

Use enum for precision #167

davebayer opened this issue Mar 13, 2024 · 2 comments

Comments

@davebayer
Copy link

davebayer commented Mar 13, 2024

Hi,

inspired by the cuFFT library, I was wandering if it would be a good idea to specify the transform precision by an enum class VkFFTPrecision defined as:

typedef enum
{
  VKFFT_PREC_UNDEF = 0,
  VKFFT_PREC_BF16,
  VKFFT_PREC_F16,
  VKFFT_PREC_F32,
  VKFFT_PREC_F64,
  VKFFT_PREC_F128
} VkFFTPrecision;

Then when configurating all of the old flags could be removed and replaced by

  • precision (default precision for other precision variables, used as forward output and inverse input),
  • forwardPrecision (forward transform execution precision),
  • inversePrecision (inverse transform execution precision),
  • inputPrecision (input data precision),
  • outputPrecision (output data precision).

At least precision variable must be always specified.

typedef struct {
  // ...

  // pfUINT doublePrecision;
  // pfUINT quadDoubleDoublePrecision;
  // pfUINT quadDoubleDoublePrecisionDoubleMemory;
  // pfUINT halfPrecision;
  // pfUINT halfPrecisionMemoryOnly;
  // pfUINT doublePrecisionFloatMemory;

  VkFFTPrecision precision;         // must be specified
  VkFFTPrecision forwardPrecision;  // optional, if is VKFFT_PREC_UNDEF, then use `precision`
  VkFFTPrecision inversePrecision;  // optional, if is VKFFT_PREC_UNDEF, then use `precision`
  VkFFTPrecision inputPrecision;    // optional, if is VKFFT_PREC_UNDEF, then use `precision`
  VkFFTPrecision outputPrecision;   // optional, if is VKFFT_PREC_UNDEF, then use `precision`

  // ...
} VkFFTConfiguration;

I think that it would really make everything more clear.

Thanks.

David

@DTolm
Copy link
Owner

DTolm commented Mar 22, 2024

Hello,

This can be done, but to have backward compatibility, I would rather do something simple, like

#define VKFFT_PREC_FP32 0
#define VKFFT_PREC_FP64 1

As for the rework of inputPrecision and so on, it is better to wait until the callback functionality is formalized and added.

Best regards,
Dmitrii

@davebayer
Copy link
Author

Hi,

so as I understand it, you suggest that the structure of the VkFFTConfiguration structure could look like:

typedef struct {
  // ...

  pfUINT doublePrecision;
  pfUINT quadDoubleDoublePrecision;
  pfUINT quadDoubleDoublePrecisionDoubleMemory;
  pfUINT halfPrecision;
  pfUINT halfPrecisionMemoryOnly;
  pfUINT doublePrecisionFloatMemory;

  VkFFTPrecision precision;
  VkFFTPrecision execPrecision;

  // ...
} VkFFTConfiguration;

where

  • precision would setup the memory precision and default precision for execution and
  • execPrecision if set would set up the execution precision.

The behaviour then would be:

  • if none of the precision variables is set, float precision is used as today
  • if any of the old precision variables (doublePrecision, halfPrecision, ...) was specified, they would have the highest preference and everything would work as today
  • if precision is specified, the memory and execution precision is set to the value
  • if execPrecision is specified, the execution precision set by precision is overrided to the given value

Do I get it right?

And I do not understand why it would be better to define the precision values as macros instead of an enum. What is the motivation?

Thank you.

Best regards
David

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

No branches or pull requests

2 participants