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

Simplification of the code by using glm and reducing templates #130

Merged

Conversation

FelixTUD
Copy link
Contributor

BREAKING CHANGES TO THE ISAAC INTERFACE

  • Removed templates for IsaacVisualization class changes expected types to be isaac internal types like isaac_float3
  • isaac now expects the render volume to be 3D, the 2D implementation wasn't working before anyway
  • Guaranteed internal types simplify the code structure
  • Removal of unnecessary templates, as they already had many requirements at the type, which often resulted in complicated compiler errors if they were not met

@@ -80,7 +80,7 @@ class TestSource1
isaac_float3 value = ptr[nIndex.x + nIndex.y * VOLUME_X
+ nIndex.z * VOLUME_X * VOLUME_Y];
isaac_float_dim< 3 > result;
result.value = value;
result = value;
Copy link
Member

Choose a reason for hiding this comment

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

IMO result is not required anymore and you can return value directly.

Copy link
Member

Choose a reason for hiding this comment

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

@FelixTUD PLease check this comment!

@psychocoderHPC
Copy link
Member

Is it possible to use the upstream version of glm for issac. If I remember correctly some changes were required to support CUDA.

@psychocoderHPC
Copy link
Member

Please add glm to the requirements in INSTALL.md. If a minimal version is required add this information too.

@FelixTUD
Copy link
Contributor Author

Yes for CUDA a recent glm upstream version works without adjustments, only the future hip support needs some changes

@FelixTUD
Copy link
Contributor Author

@psychocoderHPC the requested changes are integrated

result.value
.x = value;
return result;
return value;
Copy link
Member

Choose a reason for hiding this comment

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

Here I am not sure if you can return value only. Value is a scalar. How is the implementation if you cast isaac_float to isaac_float_dim <3>? Is the scalar only assigned to .x or to all 3 members.

Copy link
Contributor Author

@FelixTUD FelixTUD Feb 25, 2021

Choose a reason for hiding this comment

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

It is fixed now, as feature_dim is set to 1 the type is isaac_float_dim <1> and there is no implicit conversion from isaac_float to isaac_float_dim <1>

@psychocoderHPC psychocoderHPC merged commit a89bae6 into ComputationalRadiationPhysics:dev Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants