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

Write demeshening description. Fixes #2629. #3293

Merged
merged 8 commits into from May 25, 2021

Conversation

matthewturk
Copy link
Member

This adds narrative docs about demeshening and how it works from the perspective of someone using it. It includes links to the yt 4 paper description, and I've also added cross-references within the docs where appropriate.

@matthewturk
Copy link
Member Author

This should fix #2629.

@matthewturk matthewturk added demeshening Removing the global mesh for particles docs yt-4.0 feature targeted for the yt-4.0 release labels May 22, 2021
Copy link
Member

@neutrinoceros neutrinoceros left a comment

Choose a reason for hiding this comment

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

Thank you for doing this Matt ! here are a couple of questions I had while reading it.

doc/source/reference/demeshening.rst Outdated Show resolved Hide resolved
doc/source/reference/demeshening.rst Show resolved Hide resolved
doc/source/reference/demeshening.rst Outdated Show resolved Hide resolved
doc/source/reference/demeshening.rst Show resolved Hide resolved
doc/source/reference/demeshening.rst Show resolved Hide resolved
doc/source/reference/demeshening.rst Show resolved Hide resolved
doc/source/reference/demeshening.rst Outdated Show resolved Hide resolved
matthewturk and others added 2 commits May 22, 2021 09:05
Co-authored-by: Clément Robert <cr52@protonmail.com>
@neutrinoceros
Copy link
Member

Thanks for your prompt response !
I'm curious wether yt will log a message to inform the user how big the ewah files end up being ? It could be done unconditionally or maybe restricted to particularly gluttonous cases such as you describe.

@matthewturk
Copy link
Member Author

It doesn't currently log that, no. I don't think it's out of the question to do so, but I still think it's going to be unecessary. A completely uncompressed set of coarse bitarrays would take up:

(N_chunks * 2**(3*index_order1)) / 8 bytes. So for example, this means that (assuming I did my math right) a set of 1024 chunks at index_order1 of 5 (the default) would take up a little over 4 megs. Now, if we assume a 10% collision rate, this would be an uncompressed 8 gigabytes per chunk, but this is exceedingly unlikely, and also completely ignores the compression (which would drop it by a few orders of magnitude).

Looking at the data in our sample collection, it seems to me that the largest index we have is about 111M, for snipshot_399, which is roughly a sixth of the size of the full dataset. This is big, but it's not terrible, I would argue. And it's also an index_order (7, 5), which is pretty fine-grained.

Anyway, I think this is something that could be discussed, but right now I only have these heuristics to go on -- and I don't know that we want to include them if they're going to potentially be weirdly wrong or something.

@neutrinoceros
Copy link
Member

If it's possible to inflate disk usage by 17%, however unlikely, I think it warrants a log entry, albeit conditional. I've worked on clusters where exceeding my space quota by even 1MB would block any writing, so I don't think I'd be very happy if I had to go through yt's documentation to understand how that happened, most importantly because I would probably not think of yt as a suspect as I search for the reason why I keep going over my quota.

@matthewturk
Copy link
Member Author

matthewturk commented May 22, 2021 via email

@neutrinoceros
Copy link
Member

I agree !

Copy link
Member

@chummels chummels left a comment

Choose a reason for hiding this comment

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

This is really terrific work. The writing is clean and easy to read but also extremely informative. And it includes practical information on how to speed up for your own datasets. Thanks for writing this!

I included a couple of comments. The only other additional comment is something that could be included in this PR or we could work out in another PR: how does yt now treat smoothed particle datasets for the bulk of its operations? Previously, we deposited particles into octrees, and then generated things like slices and projections from that gridded data, but now we're flying free without the mesh. I think we need a base level description of how yt conducts the line integrals that traverse these smoothing kernels is warranted. But as I said, it doesn't have to be you who writes it, and it doesn't have to be in this PR, and I'm certainly open to discussion on this point. Thanks for writing this PR, @matthewturk .

doc/source/reference/demeshening.rst Show resolved Hide resolved
doc/source/reference/demeshening.rst Outdated Show resolved Hide resolved
@matthewturk
Copy link
Member Author

@chummels once you've taken a look, let me know which areas need mroe explanation, and if you feel it's OK, please go ahead and click "resolve." And if they need more work, I'll get right on it! :)

Copy link
Member

@chummels chummels 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 the additional notes--they helped clear up some of my misunderstanding. Very thorough job here. My suggestions should not hold things up, but I just noted them since you switched from "refined index" to "fine index" about halfway through. Great writeup!

doc/source/reference/demeshening.rst Outdated Show resolved Hide resolved
doc/source/reference/demeshening.rst Outdated Show resolved Hide resolved
doc/source/reference/demeshening.rst Outdated Show resolved Hide resolved
doc/source/reference/demeshening.rst Outdated Show resolved Hide resolved
@neutrinoceros
Copy link
Member

note @matthewturk , in order for #3298 to affect this PR, you'll need to rebase or merge from master

@matthewturk
Copy link
Member Author

I think the merge is done and this should be ready to go! Thanks for the review, folks.

@jzuhone
Copy link
Contributor

jzuhone commented May 25, 2021

Hi @matthewturk --

Maybe not part of this PR, but one other thing that may be relevant from the perspective of the user is how slices and projections of SPH data have changed due to the demeshening, and why these are more accurate.

I am not an expert on this stuff, otherwise I would be happy to write it myself. Maybe a couple of paragraphs would suffice?

@matthewturk
Copy link
Member Author

matthewturk commented May 25, 2021 via email

@jzuhone
Copy link
Contributor

jzuhone commented May 25, 2021

How about we just show one (the first one) and link to that page for the rest?

@jzuhone jzuhone merged commit 9e2ef5d into yt-project:main May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
demeshening Removing the global mesh for particles docs yt-4.0 feature targeted for the yt-4.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants