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

Fixed reading of IllustrisTNG data #786

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gandhalij
Copy link

Fixed the creation of offset tables to account for:

  1. Group catalogue files broken up into chunks (previously offsets were being reset for each chunk file)

  2. Multiple 'PartType's mapped onto the same familiy (previously the offsets were relative to start of the given 'PartType' block)

  3. 'Fluff' particles at the end of each FOF group, which are not associated with any Subfind subhalo

Fixed the creation of offset tables to account for:

1) Group catalogue files broken up into chunks (previously offsets were being reset for each chunk file)

2) Multiple 'PartType's mapped onto the same familiy (previously the offsets were relative to start of the given 'PartType' block)

3) 'Fluff' particles at the end of each FOF group, which are not associated with any Subfind subhalo
@apontzen
Copy link
Member

Thanks @gandhalij! Is there a sample file (preferably a nice small one) which this can be verified/tested on?

@gandhalij
Copy link
Author

Thanks @gandhalij! Is there a sample file (preferably a nice small one) which this can be verified/tested on?

I have a one of the lowest resolution TNG simulations that I can point you to. It's still fairly big, but 2 GB vs. 11.5 GB for the GM runs. Unfortunately it's not a zoom, so it won't be able to replicate the issues to do with the multiple particle types. Would that work?

@apontzen
Copy link
Member

I think there is a merge between different pynbody versions which has resulted in slightly broken code here (e.g. self.ngroups should be self._ngroups and self.nsubhalos should be self._nsubhalos.

Once this is fixed, however, it still breaks reading Gadget3/Gadget4 Subfind outputs according to the test suite. I'm unsure whether this is because of an incompatibility in the way the files are written out in arepo/TNG vs G3/G4, or something more subtle, but either way unfortunately it means there is work to be done before we can merge...

I will try to understand more what is going on at some point but @gandhalij it would be much appreciated if you can look into it too and we can try to figure out what the 'right' solution should be to read all these files correctly

@gandhalij
Copy link
Author

Ah OK, yes I think I have a much older version of pynbody that I was working on and I also don't know how different the original Gadget3/4 files are from the TNG files. I'd only tested if my version was working with the TNG files. So definitely, I'll dig into how these changes deal with the gadget files.

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 this pull request may close these issues.

None yet

2 participants