Fix GoParallelTuple tuple truncation (\#120) and Tensor2 uninitialized phonon checks (\#47)#123
Open
mesonepigreco wants to merge 2 commits into
Open
Fix GoParallelTuple tuple truncation (\#120) and Tensor2 uninitialized phonon checks (\#47)#123mesonepigreco wants to merge 2 commits into
mesonepigreco wants to merge 2 commits into
Conversation
… checks Fix #120: GoParallelTuple with mpi4py dropped all but the first element of the returned tuple due to overwriting only the first allgathered element. Replace with a proper loop that reduces each tuple element across ranks and returns the full result list. Fix #47: Tensor2.SetupFromPhonons crashed with an unhelpful AttributeError when passed an uninitialized Phonons object (no structure loaded). Add an early validation that raises a clear ValueError about the phonon not being initialized. Also add synchronized empty-work-item checks in both GoParallel and GoParallelTuple: when a rank has no work items, all ranks abort together via MPI_Abort (or raise on master) with a clear message, preventing the hang that occurred before. Tests: - test_goparalleltuple: verifies tuple elements survive serial and mpi4py paths, including across-rank reduction (mpirun -np 2 tested) - test_setup_from_phonons: verifies clear errors for uninitialized phonons and supercell mismatches
Previously the serial/mpi4py code path raised NotImplementedError when reduce_op=None, even in serial mode where no cross-rank synchronization is needed. Fixed by returning the collected per-input results directly, matching the behaviour of GoParallel (non-Tuple) which already supports this case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two open issues:
#120 — GoParallelTuple drops tuple elements with mpi4py
The mpi4py reduction path in
GoParallelTuplehad a one-line bug:result = results[0]overwrote the result with only the first element's allgathered list, silently dropping all other tuple elements. This affected any mpi4py run, including single-process.Fix: Replace the broken block with a loop that properly reduces each tuple element across ranks.
Test:
tests/TestParallel/test_goparalleltuple.py— verifies all tuple elements survive serial and mpi4py paths. Manual verification withmpirun -np 2confirmed correct across-rank reduction.#47 — Uninitialized phonon crashes Tensor2.SetupFromPhonons
Passing an empty
Phonons()(no structure loaded) toSetupFromPhononscrashed withAttributeError: 'NoneType' object has no attribute 'unit_cell'. The issue also noted supercell mismatch detection.Fix: Add an early validation check raising a clear
ValueErrorwhen the phonon object has no structure or empty q_tot.Test:
tests/TestTensor2/test_setup_from_phonons.py— verifies clear errors for uninitialized phonons and supercell mismatches.Bonus: Synchronized empty-work-item abort
Both
GoParallelandGoParallelTuplenow useMPI_COMM_WORLD.allreduce(MIN)to detect when any rank has no work items. All ranks raise together (viacomm.Abort), preventing the hang that occurred when some ranks crashed alone before a collective call.