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

Fix data type issue in _compute_doppler_shifts, fixes #426 #436

Merged
merged 1 commit into from
May 17, 2024

Conversation

Jeija
Copy link
Contributor

@Jeija Jeija commented May 10, 2024

Description

Very minor one-line fix for issue #426.
The line in the source code used to say

tf.where(objects_mask, 0, doppler)

which causes a TypeError (TypeError: Input 'e' of 'SelectV2' Op has type float32 that does not match type int32 of argument 't'.), because 0 is an int32, whereas doppler is a float32 and tf.where wants to have same dtypes (at least when using @tf.function with my TensorFlow installation).
I changed it so that the dtype of 0 is chosen based on the dtype of doppler:

doppler = tf.where(objects_mask, tf.constant(0, doppler.dtype), doppler)

Not sure if this is the correct way to fix it, but the bug certainly disappears for me.

Checklist

  • Detailed description
  • Added references to issues and discussions
  • Added / modified documentation as needed
  • Added / modified unit tests as needed
  • Passes all tests
  • Lint the code
  • Performed a self review
  • Ensure you Signed-off the commits. Required to accept contributions!
  • Co-authored with someone? Add Co-authored-by: user@domain and ensure they signed off their commits too.

Signed-off-by: Florian Euchner <florian.euchner@gmail.com>
@SebastianCa
Copy link
Collaborator

Hi @Jeija,
thank you for this pull request. We will merge it into the next release of Sionna.

@gmarcusm gmarcusm merged commit 2cb12fd into NVlabs:main May 17, 2024
4 checks passed
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