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

index_pointer dataset in example compartment reports contains an extra offset at the end #62

Open
hernando opened this issue Oct 29, 2018 · 2 comments
Assignees

Comments

@hernando
Copy link
Collaborator

@kaeldai I remember we discussed this last July in person, but since then we haven't taken any decision about what we should do. The question is: Should we update the spec to reflect that there's an extra offset at the end which matches the number of values in a frame, or should we remove it?
I would like to sort this out soon because Brion can't open this reports at the moment and we are not keen on the idea of introducing a hack for an undocumented feature in the examples.

@kaeldai kaeldai self-assigned this Nov 27, 2018
@kaeldai
Copy link
Collaborator

kaeldai commented Nov 27, 2018

I think for now it's best to just make it explicit that the extra offset is required. It will help to validate the hdf5 files in the case the data is being written in blocks but the programmer forgets to cut off the end values. Without the extra offset the software will end up reading in junk values as valid ones. I will explicit state in the doc that the extra offset is mandatory.

Juan and Judit are looking into using padding in which case this issue will not be valid anymore.

@hernando hernando assigned hernando and unassigned kaeldai Nov 28, 2018
@jplanasc
Copy link

jplanasc commented Mar 5, 2020

@kaeldai , @antonarkhipov , @jdcourcol , @mgeplf , @jorblancoa , @alkino :
This issue has been open for a long time. After internal discussion with some BBP developers, we would like to suggest that we add an extra element in the index_pointer dataset.

In any case, this is exactly what Kael implemented and it's in the current implementation of pysonata (number of elements in the dataset is total number of GIDS + 1):
https://github.com/AllenInstitute/sonata/blob/master/src/pysonata/sonata/reports/compartment/compartment_writer.py#L180

With this, the only thing that's not strictly complete is the specification (which doesn't mention this detail of the extra element). @alkino will do a PR to correct it and you can review it. So, we could also close this issue (the padding option was discarded).

Thanks!

jorblancoa pushed a commit to BlueBrain/libsonata that referenced this issue Mar 5, 2020
jorblancoa pushed a commit to BlueBrain/libsonata that referenced this issue Mar 5, 2020
alkino pushed a commit to BlueBrain/libsonatareport that referenced this issue Feb 1, 2021
alkino pushed a commit to BlueBrain/libsonatareport that referenced this issue Feb 1, 2021
alkino pushed a commit to BlueBrain/libsonatareport that referenced this issue Feb 1, 2021
alkino pushed a commit to BlueBrain/libsonatareport that referenced this issue Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants