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

CAD emulated float64 #114

Open
wants to merge 84 commits into
base: master
Choose a base branch
from
Open

CAD emulated float64 #114

wants to merge 84 commits into from

Conversation

Przemog1
Copy link
Contributor

@Przemog1 Przemog1 commented May 15, 2024

@Przemog1 Przemog1 changed the title example 22 now tests emulated_float64_t emulated_float64_t Aug 27, 2024
const emulated::float64_t a = _static_cast<emulated::float64_t>(6.9f);
const emulated::float64_t b = _static_cast<emulated::float64_t>(4.5f);

vk::RawBufferStore<emulated::float64_t::storage_t>(0,(a*b).data);
Copy link
Contributor

Choose a reason for hiding this comment

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

umm this is unsafe, you're writing to gpu virtual address of 0u, you might crash. you create a buffer and pass the address via push constant

Choose a reason for hiding this comment

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

it kinda looks like a file someone forgot to remove (no chanes in 03 main.cpp)

@@ -7,6 +7,11 @@
#include <nbl/builtin/hlsl/algorithm.hlsl>
#include <nbl/builtin/hlsl/jit/device_capabilities.hlsl>

using ActualFloat64 = portable_float64_t<jit::device_capabilities>;
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a mode/macro "ForceEmulatedFloat64" for us to test on GPUs that already have Float64 feature?
or alternatively tell me how you usually test emulated vs non-emulated rapidly?

Copy link
Contributor Author

@Przemog1 Przemog1 Oct 22, 2024

Choose a reason for hiding this comment

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

https://github.com/Devsh-Graphics-Programming/Nabla/blob/emulated_float64_t/include/nbl/builtin/hlsl/portable/float64_t.hlsl#L13

// force emulated_float64<true, true>
using portable_float64_t = typename conditional<device_capabilities_traits<device_caps>::shaderFloat64, emulated_float64_t<true, true>, emulated_float64_t<true, true> >::type;

// emulated_float64<true, true> only when shaderFloat64 not supported
using portable_float64_t = typename conditional<device_capabilities_traits<device_caps>::shaderFloat64, float64_t, emulated_float64_t<true, true> >::type;

I actually forgot to change that back, so emulated float is forced

Copy link
Contributor

Choose a reason for hiding this comment

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

then I think you can do this ForceEmulatedFloat64 option by using a fake caps struct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#ifdef NBL_FORCE_EMULATED_FLOAT_64
using portable_float64_t = emulated_float64_t<true, true>;
#else
using portable_float64_t = typename conditional<device_capabilities_traits<device_caps>::shaderFloat64, float64_t, emulated_float64_t<true, true> >::type;
#endif

did this, so in order to force ef64 one has to define NBL_FORCE_EMULATED_FLOAT_64 macro before including portable/float64.hlsl

62_CAD/main.cpp Outdated
@@ -68,7 +68,7 @@ constexpr std::array<float, (uint32_t)ExampleMode::CASE_COUNT> cameraExtents =
600.0, // CASE_8
};

constexpr ExampleMode mode = ExampleMode::CASE_7;
Copy link
Contributor

Choose a reason for hiding this comment

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

revert these changes please, I changed shader compilation a lot, it will introduce conflicts

62_CAD/common.hlsl Outdated Show resolved Hide resolved
# Conflicts:
#	62_CAD/main.cpp
#	62_CAD/shaders/main_pipeline/common.hlsl
#	62_CAD/shaders/main_pipeline/vertex_shader.hlsl
Comment on lines 43 to 48
// TODO[Przemok]: Fix/Remove your todo here?!
float2 transformPointScreenSpace(pfloat64_t3x3 transformation, uint32_t2 resolution, pfloat64_t2 point2d)
{
float64_t2 ndc = transformPointNdc(transformation, point2d);
return (float32_t2)((ndc + 1.0) * 0.5 * resolution);
pfloat64_t2 ndc = transformPointNdc(transformation, point2d);
// TODO: "ndc + 1" doesn't work, probably because of `emulated_float64_t::create(float val)`
pfloat64_t2 result = (ndc + 1.0f) * 0.5f * _static_cast<pfloat64_t2>(resolution);

Choose a reason for hiding this comment

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

  • int shouldn't work, I don't like it

and still because HLSL doesn't do conversion operators you're unable to get an implicit conversion to satisfy an argument type.

In C++ double+int compiles because double has an implicit constructor from int and only operator+(const double)

NOT because an overload of + exists for every other type (float, int, etc.) as that would wreck mayhem everywhere else (every function that expects a double would throw a fit if you try to feed an integer into it.

What I want to avoid is double+int working and then people asking why f(double) doesn't work when you call with f(int)

Comment on lines 18 to 31
// for initial seed of 69, the polynomial is:
// f(x) = x^15 * 187.804 + x^14 * 11.6964 + x^13 * 2450.9 + x^12 * 88.6756 + x^11 * 3.62408 + x^10 * 11.4605 + x^9 * 53276.3 + x^8 * 16045.4 + x^7 * 2260.61 + x^6 * 8162.57
// + x^5 * 20.674 + x^4 * 13918.6 + x^3 * 2.36093 + x^2 * 8.72536 + x^1 * 2335.63 + 176.719
// f(1) = 98961.74987
// int from 0 to 69 = 3.11133�10^30

template<typename F64>
struct Random16thPolynomial
{
void randomizeCoefficients()
{
Xoroshiro64Star rng = Xoroshiro64Star::construct(69);

// can't just do `coefficients[i] = rng()` this will create retarded numbers or special value exponents
Copy link
Member

Choose a reason for hiding this comment

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

language and innuendo son.

Get rid of the 69 and "retarded"

Copy link
Member

Choose a reason for hiding this comment

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

P.S. I'm sure the numbers would be "fine" if you wiped the position 62-60 bits to 0

@devshgraphicsprogramming
Copy link
Member

Has my seal of approval

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.

3 participants