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 for function from_meshio #3049

Merged
merged 4 commits into from Jul 24, 2022
Merged

Fix for function from_meshio #3049

merged 4 commits into from Jul 24, 2022

Conversation

keurfonluu
Copy link
Contributor

Overview

  • Fixed: function pyvista.from_meshio when cells data type is np.uint64.

Details

The following code raises an error:

import pygmsh
import pyvista

with pygmsh.geo.Geometry() as geom:
    geom.add_polygon(
        [
            [0.0, 0.0],
            [1.0, -0.2],
            [1.1, 1.2],
            [0.1, 0.7],
        ],
        mesh_size=0.1,
    )
    mesh = geom.generate_mesh()

mesh = pyvista.from_meshio(mesh)

This error is due to np.hstack somehow returning an array of type np.float64 when input array data type is np.uint64:

x = np.array([[0, 1], [2, 3]], dtype=np.uint64)
y = np.hstack((np.full((len(x), 1), 0), x))
print(y.dtype)

Output:

float64

@codecov
Copy link

codecov bot commented Jul 22, 2022

Codecov Report

Merging #3049 (e8c65bb) into main (d3e01ee) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3049      +/-   ##
==========================================
- Coverage   94.39%   94.38%   -0.02%     
==========================================
  Files          76       76              
  Lines       16556    16566      +10     
==========================================
+ Hits        15628    15635       +7     
- Misses        928      931       +3     

@adeak
Copy link
Member

adeak commented Jul 22, 2022

Thanks for the PR and finding this!

This error is due to np.hstack somehow returning an array of type np.float64 when input array data type is np.uint64:

This is because of this:

>>> x = np.array([[0, 1], [2, 3]], dtype=np.uint64)

>>> x.dtype, np.full((len(x), 1), 0).dtype
(dtype('uint64'), dtype('int64'))

np.full sees native python int 0 as the fill value, and creates a numpy default int (int64 on 64-bit linux/unix, int32 roughly everywhere else). But you can't reconcile int64 and uint64 values in the same int array, because they have different ranges. In these cases NumPy switches to doubles, with the caveat that one might lose precision for very large numbers:

>>> (np.int_(1) + np.uint64(1)).dtype
dtype('float64')

I'm wondering if we should also change the hstack line, to always match the dtype of the input. That way we can avoid type promotion on the np.hstack line.

@akaszynski akaszynski added this to the 0.35.3 milestone Jul 23, 2022
Copy link
Member

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

Thanks for adding in the 2D test @keurfonluu. This LGTM, and as a side benefit we've marginally improved coverage.

@tkoyama010 tkoyama010 added the bug Uh-oh! Something isn't working as expected. label Jul 23, 2022
@akaszynski akaszynski merged commit a176f2d into main Jul 24, 2022
@akaszynski akaszynski deleted the meshio branch July 24, 2022 23:00
akaszynski added a commit that referenced this pull request Jul 30, 2022
* ensure cells are int64

* check dtype

* test from_meshio with 2d mesh

* fix isort

Co-authored-by: Alex Kaszynski <akascap@gmail.com>
This was referenced Jul 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Uh-oh! Something isn't working as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants