Merged
Conversation
…ally, then convert to numpy function for vectorized calculation.
…t from the paper, the sqrt(1-k^2) is actually 1-k^2. Now fix it
…celerate the calcution 4 times faster
2. add english comments to the code 3. fix the singularity problem when x^2+y^2=0 for Bzxx 4. use sympy. to replace sp. To do: There might be another way to convert the sympy expression to a function: autowarp, need to try out.
…o I have sloved a bug in the magnet_cylinder.py. -Now I am currently using 11 cuboids to approximate a cylinder. When viewed from the vertical direction, 11 rectangles are arranged side by side to simulate a circular shape. After testing, the relative error in regions with strong signals is less than 1%, and the runtime has been reduced by nearly 100 times. I have also written the corresponding test files. -I discovered a bug in the old CylinderMagnet class: I had forgotten to account for the offset of the z-coordinate relative to the origin. This issue has now been fixed.
In the polarization function, the prolarization when x=0(relative to the center of the cylinder) is not nan due to we are calcutating 0/0. We used to think there gonna be all nan, however, it turns out to be incorrecte. So now the function will find the yz slice that has nan and replace it with the average of 2 slices new to it. I also added a test for the polarization function. There is another way to fix it, I calcuate the limit of the polarization function when x->0, but that need Bzxx, maybe I can write a new version in the future.
…oxByRectangularMagnet
… returns NaN. Initially, I thought this was due to a 0/0 division in the middle of the x-axis. However, this issue can occur anywhere when the tip-sample separation is too small. Now, in this function, it scans over the x-axis, identifies all NaN values, and replaces them with the average of their neighboring values.
… into FastCermitted
CylinderMagnet.py use analytical solution to calculate the magnetic field, which is too long to calculate, so we decide to just use the approximate method.
The old test rely on CylinderMagnet, which is deleted now
Cylinder magnet
These 2 files are no longger useful
Added ``CylinderMangetApprox`` class that approximates a cylindrical magnet. Modified ``rel_dpol_sat_td`` with updated Nan value treatment.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces version 0.4.0 with significant improvements to handling NaN values in polarization calculations, documentation enhancements, and the addition of a cylinder magnet approximation class.
- Enhanced NaN handling in
rel_dpol_sat_tdfunction with proper validation and error reporting - Added
CylinderMagnetApproxclass to approximate cylindrical magnets using 11 rectangular magnets - Updated dependency from git-based mmodel to published version >=0.9.0
- Comprehensive docstring improvements with standardized formatting and units
Reviewed Changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| mrfmsim/formula/polarization.py | Improved NaN handling in rel_dpol_sat_td with boundary checks and better error messages |
| mrfmsim/component/cylindermagnet.py | New class implementing cylinder magnet field calculations via rectangular magnet approximation |
| tests/test_formula/test_polarization.py | Added comprehensive tests for NaN handling scenarios |
| tests/test_component/test_cylindermagnet.py | Complete test suite for cylinder magnet symmetry and field accuracy |
| pyproject.toml | Version bump to 0.4.0 and mmodel dependency update |
| mrfmsim/component/init.py | Exported new CylinderMagnetApprox class |
| Multiple documentation files | Fixed typos, improved formatting, and enhanced clarity |
| docs/contribute.rst | New contribution guidelines document |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
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.
Version 0.4.0
Type of Pull Request
Changes Made
Added Cylinder Magnet and minor fixes to docstrings and documentation.
Testing Performed
toxorpytest)Documentation
cd docs && make html)Checklist
Blackformatter's default settings)