Skip to content

fix sync of eigenmode calc when no mode is found#596

Merged
stevengj merged 4 commits intomasterfrom
eigmaster
Nov 9, 2018
Merged

fix sync of eigenmode calc when no mode is found#596
stevengj merged 4 commits intomasterfrom
eigmaster

Conversation

@stevengj
Copy link
Collaborator

@stevengj stevengj commented Nov 6, 2018

This is an improved version of #594 that skips most of the eigenmode calculation on non-master processes (#595), which fixes a potential deadlock if the processes didn't agree in the case where no mode is found.

Also, it fixes a memory leak when no mode is found (returning NULL). It also gets rid of the rand_r call from #568, which is no longer needed since H is only initialized on the master process.

@coveralls
Copy link

coveralls commented Nov 6, 2018

Coverage Status

Coverage increased (+0.4%) to 82.408% when pulling 013f21a on eigmaster into 97396af on master.

@stevengj
Copy link
Collaborator Author

stevengj commented Nov 7, 2018

@ChristopherHogan, it seems like the MPI Python test is hanging on Travis. What do I have to do to reproduce this locally? make -C python check doesn't hang for me, but I'm not sure whether it's using multiple processes.

@ChristopherHogan
Copy link
Contributor

make check uses 2 processes when Meep is built --with-mpi. It looks like binary_grating.py is the hanging test, so you could try running it manually with mpirun -n 2 python path/to/python/tests/binary_grating.py.

@stevengj
Copy link
Collaborator Author

stevengj commented Nov 9, 2018

Looks like it is fixed now. I'll merge once the latest CI tests are green.

@stevengj stevengj merged commit a73cfb9 into master Nov 9, 2018
@stevengj stevengj deleted the eigmaster branch November 9, 2018 21:09
bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
* fix sync of eigenmode calc when no mode is found

* sync k, vgrp

* whoops, save eigval before deleting eigvals array

* compute vgrp even if !match_frequency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants