Skip to content
This repository was archived by the owner on Jun 11, 2025. It is now read-only.

feat: Add method to convert global ids to local ids for PlacementSet#40

Merged
Helveg merged 9 commits intodevfrom
feat/get_locals_id
Apr 12, 2025
Merged

feat: Add method to convert global ids to local ids for PlacementSet#40
Helveg merged 9 commits intodevfrom
feat/get_locals_id

Conversation

@filimarc
Copy link
Contributor

@filimarc filimarc commented Feb 18, 2025

Since the need to deal with global ids for targetting classes i add a method to convert ids called: convert_to_local.

it is linked to the PR in bsb-core: dbbs-lab/bsb-core#898

  • Add test

@Helveg
Copy link
Contributor

Helveg commented Feb 19, 2025

Can you add some tests so that I can debug how your algorithm works? It still looks way too complicated with way too many array lookups like np.isin etc for what it should be doing. In my mind it should:

For every element in ids, find the chunk it belongs to, use the chunk offset + cell-in-chunk offset to get the local id.

That should not take 10+ array operations to achieve, so I'd like to double check what's going on in your array operations :)

@filimarc
Copy link
Contributor Author

filimarc commented Feb 19, 2025

I try to refactor to follow your logic, still i haven't found a way to filter ids for local chunks, to prevent to convert ids not related to that chunk and also to compute the local chunks offset, without using lookups. Anyway i am using isin only for chunk id lists, so should be an order of thousands of elements, i think it should be fast.
But maybe you have a better solution for that, if you think it could help i will add some tests.

@filimarc filimarc requested a review from Helveg March 17, 2025 17:12
@drodarie
Copy link
Contributor

drodarie commented Apr 1, 2025

@Helveg, Filippo implemented your feedback. Could you please review this PR, we would like to bump the main bsb package.

@filimarc filimarc changed the base branch from main to dev April 10, 2025 12:20
@Helveg Helveg merged commit a141f74 into dev Apr 12, 2025
11 checks passed
@Helveg Helveg deleted the feat/get_locals_id branch April 12, 2025 15:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants