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
New implementation to read and write VTK files #129
Conversation
- correct component labels - different shape
Codecov Report
@@ Coverage Diff @@
## master #129 +/- ##
==========================================
+ Coverage 95.46% 95.69% +0.23%
==========================================
Files 20 20
Lines 1986 2046 +60
==========================================
+ Hits 1896 1958 +62
+ Misses 90 88 -2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good, I have added a few comments
This saves time compared to lazy computation (for the first call) if the mesh is not too small and in any case makes all subsequent call much faster.
return vertices(*(np.linspace(pmin, pmax, n + 1) for pmin, pmax, n in | ||
zip(self.region.pmin, self.region.pmax, self.n))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marijanbeg If you are happy with this (not anymore lazy) calculation of the vertices
/midpoints
we can merge the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lang-m, could you please remind me why we decided this not to be lazy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Copied from Zulip, looks like you've missed it there, and slightly extended]
I think using np.linspace
would be a lot better, because:
- it is generally more convenient to use (no need to convert to a list first) and all parts within
discretisedfield
can use/need a numpy array "version" anyways. - faster for larger n (example: 2000 cells, generator: 750μs, numpy: 200 μs with the additional calls to the mesh/region for (smaller) numbers of cells in the other two directions); subsequent calls are ~50 ns (due to caching).
- easier to use because we can assign and reuse it, whereas more care is necessary with the generator expression.
<https://docs.enthought.com/mayavi/mayavi/>`_. | ||
<https://docs.enthought.com/mayavi/mayavi/>`_. To show contour lines in | ||
Paraview one has to first convert Cell Data to Point Data using a | ||
filter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding that explanation - this will be useful for people ;-)
No description provided.