Proposal change type of `Range`.

[1uc]

Currently, we define https://github.com/BlueBrain/libsonata/blob/7a3983a4ac89535550075feafd5338a1693a1f5f/include/bbp/sonata/population.h#L33

and then we define a RawIndex here: https://github.com/BlueBrain/libsonata/blob/7a3983a4ac89535550075feafd5338a1693a1f5f/src/edge_index.cpp#L27

semantically the two are identical, i.e. a list of ranges. The issue is that HighFive doesn't support std::pair. There's likely two issues with supporting std::pair: a) I can't find any guarantee which order the two members of the struct are in, b) there might be padding (similar holds for std::array). Therefore, we have two datastructures, and are forced to copy.

The proposal would be, since libsonata doesn't have a stable API (yet), we might want to use:

using Range = std::array<Value, 2>;   // Value == uint64_t

[mgeplf]

Yup, makes sense. We'd have to run it by the viz team, to see if it breaks any of their code, though. Perhaps @jplanasc / @georgeskhazen can suggest someone - maybe @Adrien4193?

since libsonata doesn't have a stable API (yet)

I try pretty hard to not break things...

[Adrien4193]

From my side, I don't see any issues changing that.

If it breaks anything in Brayns we should see it in the spack build and I can make a quick release if changes are needed.

[1uc]

Probably easier to make the change and then prepare to deploy it. Spack CI will compile pretty much everything that runs on BB5. One could hope that covers everything developed (seriously) at BBP.

One trick that makes this a little more palatable is that one can easily write code that compatible with version of libsonata using std::pair and std::array by using std::get.

[mgeplf]

One trick that makes this a little more palatable is that one can easily write code that compatible with version of libsonata using std::pair and std::array by using std::get.

Yeah, that works well.

My suspicion, based on nothing, is that most places don't inspect the ranges, so it's probably fine.

@1uc have you already made the change somewhere?

[1uc]

No, I've kept any changes agnostic of the difference whenever needed; but it would be nice to scrap all that.

[mgeplf]

cool; I'll try it quickly, and see what it's like. Great idea, btw.

[1uc]

PR: #319 and "integration test": https://github.com/BlueBrain/spack/pull/2216

[mgeplf]

@Adrien4193 can you try the above PR, and see if it works with Brayns? Thanks.

[Adrien4193]

I will do it once the spack build is finished and let you know.

[1uc]

Only synapsetool and touchdetector fail with the change.

[Adrien4193]

I there a way to still test Brayns or it requires that everything has built ?

[1uc]

You can use "editable" Spack, clone the branch and build Brayns in a spack env. Then perform whatever testing you like. I don't think any modules were created if that's was what you wanted to use.

[Adrien4193]

Then just go like that and I will test it once deployed.

I don't see any reason it could break Brayns but if it does, I will fix it on Brayns side.

[mgeplf]

Implemented in: https://github.com/BlueBrain/libsonata/pull/319 Thanks @1uc