-
Notifications
You must be signed in to change notification settings - Fork 626
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
WIP: loop_in_chunks over owned points #1895
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
You might want to rebase (or even pull and force push). |
FYI I tried merging this branch with #1855 and ran a simple adjoint test case both with and without mpi (no symmetries). The fields blew up in both cases. |
I am still having some difficulties. For concreteness, let's focus on the first test case in symmetry.cpp: To make sure we are on the same page, here is the situation:
Using symmetry:
Issue: In If I change the start location and size of the halved volume, either the boundaries are still "doubly double counted", or not counted at all. Moreover, the halved chunk isn't really half the original chunk, which may be potentially problematic. |
By the way, using metallic BC in the same test case, without using symmetry, the overlap will be 2 to 32. Although both chunk and |
As discussed: when you nontrivially ( |
As discussed today, you only want to remove the symmetry-plane overlap from a transformed chunk in a direction where that transformed gv is the "lower half" of the original user_volume. For any halved direction, this is determined by whether the |
It is almost working now. Here are the current issues when running CI:
|
I test your branch using an example almost the same as that in #1555. The results show that the issue still exists and is even more obvious. The code is as follows, which is based on your example code.
I am sorry that I did not test it earlier. My only computer broke down and was left for repair in the last few days. |
@mawc2019 There seems to be an issue with overlapping design regions #1984. The adjoint gradients in the code are simply incorrect. The gradients are correct after removing this line |
This commit sometimes causes incorrect adjoint gradients in the
but after this commit yields
|
I didn't get same results with your exact code, but the gradients are indeed different. And in my case, the finite-difference gradients also changed somehow.
after:
However, I personally don't think there is an issue with this PR. For near2far, the accuracy is actually improved overall (the only outlier is when the value itself is close to 0):
after:
|
I am using Python 3.7. Which version of Python are you using? Did you run the code on your own computer or Supercloud? |
Python 3.9, run locally on my laptop. |
I reran the test with Python 3.8 in SuperCloud, and the results are exactly the same as my previous results using Python 3.7. You may also rerun the test in Supercloud when you have a chance. In your previous results, one of the finite-difference gradients become exactly zero after this PR. Maybe that is not something usual:
|
I just did, and the results are actually similar to what I have locally. I tried other cases as well, and the conclusion is consistent with my previous comment: except in your original test case, the gradients don't change much before and after this commit. At least from my perspective, there isn't any issue with this PR. You should try other cases to see whether the adjoint gradients are always worse for you. |
I just reran the test with the same setup, but this time, I output the gradients with respect to all design pixels and calculated the relative error as I tried another setup, and the results are similar. Besides, the adjoint gradients in the
The relative difference between 1 process and 8 processes is calculated as In addition, sometimes the adjoint gradient blows up. Here is an example where the design region does not touch the cell boundary.
When running with 16 processes, the code yields the adjoint gradients as |
I tested this as well previously with your code. The adjoint gradients actually agreed with different numbers of processes after this PR but not before. In your plot, notice that the relative differences are now at 1e-5, while in #1555 they went up to almost 1e+2.
I tried the code on Supercloud and actually didn't see any blowup in gradients or any warning. |
This difference is indeed small but much larger than the machine precision, so something unusual still seems to be in the code. I also have seen a
Did you use 16 processes? This problem probably does not appear with a small number of processes. Maybe we need to confirm that our results are the same at first. |
I would argue that the differences are generally much smaller than before. There might be other places that contribute to the differences, but not necessarily an issue with this PR.
I did, and it worked fine. |
We don't expect the difference to reach machine precision. A relative error of 1e-5 is actually pretty good. |
That is possible. Probably this PR itself is perfect, but still does not suffice to completely resolve the problem.
Some other examples may give much more obvious differences. For example, in the previous case, the adjoint gradient is normal at 1 process but diverges at 16 processes:
However, Mo did not see the same thing. |
It seems that Meep still has the same issue as #1555, although it may become moderate. Here is another Among 2, 4, 8, and 16 processes, the adjoint gradients show negligible differences. But at 32 processes, the differences become obvious again. Here is the relative difference between 16 and 32 processes, i.e., For finite-difference gradients, the relative differences among different numbers of processes are negligible, which are around 1e-10. The code is as follows.
|
It seems that this PR also works for eigenmode coefficients, but the results between different numbers of processes can still be noticeably different. Some 3d tests for |
Fixes #1555.