-
Notifications
You must be signed in to change notification settings - Fork 280
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
octree total gas masses improperly inversely scaling with nref #4818
Comments
Hi, and welcome to yt! Thanks for opening your first issue. We have an issue template that helps us to gather relevant information to help diagnosing and fixing the issue. |
Hi! I think this is because the masses shouldn't be smoothed onto the octree, but rather the density, and then that multiplied by the volume. Can you check those results? That being said, even if that produces the correct answer, this behavior should indeed be flagged in the output so that this doesn't cause issues in the future. |
Hi! I'm not 100% sure that I did this correctly, so just pasting the volume code I added to the loop in the above script in case I messed up:
Assuming that this is right, this is the output I got for the ratios between the total octree mass and the total particle mass:
This is obviously not increasing as strongly with nref, but it is, and never seems to be about 1. |
@DhruvZ this is a bit of a lark but: i'm actually having trouble loading the octree without defining a left edge and a right edge. i wonder if this owes to differing versions of yt or something. i wonder if loading like this:
makes any difference? |
I managed to get the same output loading the octree as you described, unfortunately. |
Based on pixelization_routines.pyx, Is there any chance that the octree is using the |
Hi, thanks for your thoughts! I'm not sure what the most efficient way to check if a was function was called in a package during a script - is there a cleaner way to do it than using |
I mean, my idea was the even more basic 'add a print statement to the function' |
I've been playing around with adding a print statement to the function (and the other related functions), and I'm feel like I'm potentially doing something wrong, but I'm not sure what. I added print statements to the source and reinstalled, but I don't see them out anywhere when running. I do see them where they should be in the compiled code. I also made a new environment for testing this and no change again. This seems to imply that neither of the methods are being called in the script I posted above (either in the original mass-based version or the updated density one). |
... ok, that's weird. It should be handled by the SPH backend, but maybe the octree stuff has its own routines? I'm not too familiar with those. Do you see the print statements if you add them to the very first thing that gets called? That might tell you if the issue is with the installation or the assumed functions. I'm trying to trace what gets called here, but that's proving to be a bit tricky. I think ds.octree(n_ref=nval) calls the |
I just added a print statement to the |
Hey, I know it's been a while, but those fixes for the |
Bug report
yt (gas) octree smoothing does not conserve mass, and the discrepancy from the true particle mass total seems to scale inversely proportional to nref. This test uses the gizmo_mhd_mwdisk example file.
Code for reproduction
Actual outcome
The total octree grid mass is scaling inversely with the refinement parameter, as the last output line indicates. This should ideally be staying roughly constant at a ratio of 1 as nref changes. Gather smoothing is not necessary to get this behavior in the output. I also created a fresh yt 4.3 environment (python 3.10 again) and installed with pip and was able to reproduce this behavior.
Version Information
I installed yt from source to get this output. However, I also tested in a fresh python 3.10 environment with the pip install today and was able to reproduce this scaling total mass behavior. I first encountered this issue with my own dataset (from a SIMBA run, which uses GADGET) before I attempted on this example snapshot. I also got this output by manually setting region boundaries to +-400 rather than letting the auto region work.
The text was updated successfully, but these errors were encountered: