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

refactor!: mpi hdf5 #36

Merged
merged 9 commits into from
Jan 7, 2025
Merged

refactor!: mpi hdf5 #36

merged 9 commits into from
Jan 7, 2025

Conversation

drodarie
Copy link
Contributor

@drodarie drodarie commented Dec 17, 2024

Add MPILock to static methods to load hdf5 file.

Fix dbbs-lab/bsb-core#893
Needs dbbs-lab/bsb-core#902

Closes #16

@drodarie drodarie requested a review from Helveg December 17, 2024 12:09
@drodarie drodarie self-assigned this Dec 17, 2024
@github-actions github-actions bot added the fix label Dec 17, 2024
@Helveg
Copy link
Contributor

Helveg commented Dec 21, 2024

Neither this or the parent PR are fix PRs, they're a breaking change to the storage interface. Maybe this is a good time to include a dev doc in the public documentation that documents all the pieces of the Storage interface, and to see if there's other opportunities of stuff to rectify. I'd for sure make sure that all method signatures are sort of the same, and that all the methods are MPI aware (since that seemed to have been overlooked for some?) and have access to the communicator.

Copy link
Contributor

@Helveg Helveg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is a breaking change, needs an extra moment to see if there's other problems

@drodarie
Copy link
Contributor Author

drodarie commented Dec 28, 2024

I do not understand why we do not simply clear the chunks' group in the hdf5 file when we clear their content.
In Chunkloader.clear.

Is there a reason why they should not be removed for good?

    @handles_handles("a")
    def clear(self, chunks=None, handle=HANDLED):
        if chunks is None:
            chunks = self.get_loaded_chunks()
        for chunk in chunks:
            del handle[self.get_chunk_path(chunk)]

@drodarie drodarie changed the title fix: mpi hdf5 refactor!: mpi hdf5 Dec 29, 2024
@Helveg
Copy link
Contributor

Helveg commented Dec 31, 2024

Is there a reason why they should not be removed for good?

I don't recall, you can give it a go and see if anything breaks.

@drodarie drodarie merged commit 7a89d49 into main Jan 7, 2025
11 checks passed
@drodarie drodarie deleted the fix/mpi-hdf5 branch January 7, 2025 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loading multiple time the same file with MPI Clear chunk size
2 participants