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

Grid.isosurface for complex values? #709

Closed
zerothi opened this issue Mar 14, 2024 · 5 comments · Fixed by #711
Closed

Grid.isosurface for complex values? #709

zerothi opened this issue Mar 14, 2024 · 5 comments · Fixed by #711

Comments

@zerothi
Copy link
Owner

zerothi commented Mar 14, 2024

Describe the bug

Currently when doing Grid.isosurface it truncates the complex part. So it might be rather difficult for end users to see this.

scikit-image is (I think) keeping the floats, and it might be a long time to allow complex values scikit-image/scikit-image#4308

@zerothi
Copy link
Owner Author

zerothi commented Mar 14, 2024

@pfebrer should we warn about complex data-type, and then only pass the float, or simply accept the warning that arises?

@pfebrer
Copy link
Contributor

pfebrer commented Mar 14, 2024

Hmm interesting, do complex value iso surfaces even make sense? Can you say that some complex value is larger/smaller than another? You would have to compute some real number out of them (e.g. the modulus) and then compute an isosurface for that, right?

If that's the case, I think a warning from our side wouldn't solve much. We could either keep the scikit-image warning or raise an error (which maybe is the best option).

@zerothi
Copy link
Owner Author

zerothi commented Mar 15, 2024

Yeah, probably the modulus would be the best thing, shouldn't we then silently do that for a complex grid?
In that case I would do something like this:

  1. convert the grid to the modulus
  2. define a projection angle (lets default to 0, real axis choice), then choose the sign of the modulus depending on the projection?

Or would you do something else?

@pfebrer
Copy link
Contributor

pfebrer commented Mar 15, 2024

I'm not sure if we should do anything silently 😅 In any case, I would do it only if the isosurface level requested by the user is also a complex value, so that we are more certain that this is what the user wants to do.

Otherwise I would raise an error.

@zerothi
Copy link
Owner Author

zerothi commented Mar 15, 2024

I'm not sure if we should do anything silently 😅 In any case, I would do it only if the isosurface level requested by the user is also a complex value, so that we are more certain that this is what the user wants to do.

Otherwise I would raise an error.

Asking for a complex isosurface would be horrendous :)
It will be individual points, I think perhaps the modulus would be the most appropriate.

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 a pull request may close this issue.

2 participants