Permanent magnet branch merge #296
Merged
Conversation
This file contains 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
… Working on building the grid for the dipoles.
…i quadrature points.
…but learning how to use the code so hopefully is worth it. Still some weirdness going on with the surfaces. Next steps: fix and plot the three surfaces (plasma, inner PM surface, outer PM surface). Then build uniform (r,z) bricks and just loop through and remove any that dont fit between the surfaces. Save list of the centers of the bricks.
…oordinates. Got the grid working between the two toroidal surfaces. Two toroidal surfaces currently being generated by extend_via_normal times some offset value, defaulting to ~10 cm.
…ned up the code and the permanent magnet class, moved it to a new directory.
…al vector that keeps all the points in the same original toroidal plane. Added the MwPGP algorithm from another python project. Fixed the overall geometric factor for creating the A_obj matrix in the optimization. Still having issues fully getting the grid points inside the two toroidal surfaces -- there seems to be small errors when phi approaches 0.25 (pi / 2) and going to investigate this more soon. For now, next step is to solve the convex version with MwPGP and then do some sanity checks, poincare plots, etc.
…ixed some errors in the PM optimization, one of which was that the dipole maxima was not being computed (at all) and then I was using the maxima instead of the maxima **2 in a number of places. Still some work to get the optimization fully working but appears to be doing somewhat reasonable things so far. Things looking pretty good. Next step is to build a MagneticField class for the permanent magnets.
…eticField class. Will debug this thoroughly next week.
…ncare plotter. However, looks like the computation is too slow currently. Working on speeding this up. Also added option to truncate small elements of the linear least squares matrix in the permanent magnet optimization problem.
…s in ATA instead of in |ATA| so all the negative terms would be chopped. Changed _optimize so that all the optimization-specific parameters do not get saved to the permanent magnet object. This is so we can rerun the optimization with different parameters but the same underlying magnet configuration. Added a file (that is in progress) for scanning the hyperparameters.
…nished and not even clear this will work but figure it is worth spending a full day or two to try and see if can get significant speedups with this.
…de a complete mess right now. Need numpy-like operations in the C++ code so added the xtensor-blas library as a submodule. I think I did this basically right but not quite compiling correctly.
…n or C++ for the poincare plots.
…ause I rewrote the dipole field calculation in c++ using simple for loops. Seems to work (added some unit tests) but still appears to be excruciatingly slow in the poincare plot calculations. Not sure what to be done about that. Moved the Dipolefield class to magneticfieldclasses.py, like the Dommaschk class. Will delete the older dipolefield.py file on next commit.
…opefully will be worth the time spent when it is working. So far have written a minimal projected gradient version which is at least running without errors (when openmp off). Lots of work still to do.
…n algorithm. Still working on it.
…t almost certainly not optimized. Will optimize and clean this up dramatically in the next few days. Also did some linting fixes. The issues I was seeing earlier were related to (1) initializing/setting xtensors inside openmp loops, or (2) doing the same on shared variables (like doubles) so just need to declare these variables as private.
…Now prints out all the errors every 10 iterations. Looks like it works well so far, but have only tested with the various regularizers set to zero.
…ll allows one to switch between the python and c++ versions (for now). Seems to be working well with the L0 norm, although the L0 value is not printed correctly yet. Committing this version now in case I go ahead and cut the python routine + otherwise significantly reduce the python code.
…ld be private. Unfortunately poincare plots still not looking right despite the low Bn errors after optimization.
…o the symmetries. Also found some phis that were missing factors of 2pi... now the optimization is performing much stronger (and hopefully better). The poincare plots appear to be showing something now.
…al angle 2pi values. Fixed B and dB dipole calculations to correctly use private variables. Still having some weird issues, and can no longer compile the code with python setup.py develop (or pip) for some reason. Going to save my progress and try compiling with the main branch.
mbkumar
requested changes
Apr 28, 2023
There was a problem hiding this comment.
I have a few comments.
- Can you please introduce type hints and minimize the if-else checks on the arguments?
- Check the alignment of docstrings. Don't indent them further than the indent of """.
- This PR and Non-XSIMD implementation of Biot Savart kernel for PowerPC #292 don't go together. We need non-XSIMD implementation for the XSIMD-based C++ code added here. First, let's merge this and then we can update Non-XSIMD implementation of Biot Savart kernel for PowerPC #292.
|
I'll take one more pass at this PR sometime this weekend. |
landreman
reviewed
Apr 28, 2023
|
Recently, simsopt port without XSIMD dependency is merged into master. A new test is added to check against this. Make sure your C++ code can work without XSIMD. If required I can take a stab at this. |
|
Integrated tests (aka examples) are taking more than 3 hours. Can you find a way to reduce the execution time? We limit the iteration numbers and adjust the stopping criteria to speed up the execution in CI. You can leave notes and commented-out code in the examples for users to run at full scale. |
…d Bharat in latest round. Added some comments. Refactored the PermanentMagnetGrid so that if initialized from famus, dont need the rz_inner_surface and outer surface objects, which makes more sense. Converted some of the if statements to type casts. Going to merge with latest main and recheck all the tests now.
…d the resolution in test_pm_grid. Going to push this now and see if it works in the online CI.
…ne suddenly. Lets see what online CI does now.
…. Hoping this all passes now and runs much quicker. Required downsampling to be implemented in the two ways to load FAMUS files, since optimizing big grids from FAMUS looked like the main bottleneck.
…the FocusData class.
|
Hi all, I think I have addressed the various comments/changes and things are passing now. Let me know if there are last changes and thanks for your help! |
mbkumar
requested changes
May 23, 2023
|
Pushing up your request changes shortly, thanks! |
…ed changing the initialization of the PermanentMagnetGrid to using geo_setup functions as classmethods, adding comments, using pathlib wherever possible, among other various comments. Updated all the examples and tests to work with the new grid initialization.
mbkumar
requested changes
May 24, 2023
|
Pushing up these changes in a moment, thanks! |
|
Looks like only a docker error, related to VMEC installation (maybe I broke something on my recent push to VMEC2000/ that added config files for Perlmutter?) remains. Let me know last steps and thanks for the help! |
mbkumar
approved these changes
May 25, 2023
mbkumar
approved these changes
May 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Hi all,
The permanent magnet branch is looking pretty ready for a merge into the main branch. Some known issues/idiosyncrasies:
Looking forward to any other thoughts, and thanks for the help!