Skip to content

FIX: Correct unit for h-field in set_non_linear() for bh curve definition - #6156

Merged
jvela018 merged 8 commits into
mainfrom
fix/set_non_linear
May 14, 2025
Merged

FIX: Correct unit for h-field in set_non_linear() for bh curve definition#6156
jvela018 merged 8 commits into
mainfrom
fix/set_non_linear

Conversation

@jvela018

Copy link
Copy Markdown
Contributor

Issue linked

#6154

@ansys-reviewer-bot

Copy link
Copy Markdown
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@github-actions github-actions Bot added the bug Something isn't working label May 14, 2025
@jvela018
jvela018 requested a review from Samuelopez-ansys May 14, 2025 13:49

@Samuelopez-ansys Samuelopez-ansys left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I fixed a bug last week because they were changed, but I introduced another bug.

Please could you add a test to avoid this? Maybe you can add something similar to what it triggered you to find the bug

@jvela018 jvela018 changed the title FIX: Correct unit for b-field in set_non_linear() for bh curve definition FIX: Correct unit for h-field in set_non_linear() for bh curve definition May 14, 2025
@jvela018

Copy link
Copy Markdown
Contributor Author

@Samuelopez-ansys
I added an asser to check that b and h have defined units. However, I noticed because it defaulted to none. I didn't add the cases where they're none because before defining them they could be None and I don't want them to fail. However, the units can probably be changed so maybe at some point they could be generalized so that they take for example uTesla and so forth. Then again, I guess we can modify the units after defining the material. In this case this wouldn't be affected at all.

@jvela018
jvela018 requested a review from Samuelopez-ansys May 14, 2025 14:19
@codecov

codecov Bot commented May 14, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.22%. Comparing base (7d27b37) to head (23be064).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6156   +/-   ##
=======================================
  Coverage   85.22%   85.22%           
=======================================
  Files         167      167           
  Lines       63737    63737           
=======================================
  Hits        54323    54323           
  Misses       9414     9414           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MaxJPRey MaxJPRey left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jvela018
jvela018 enabled auto-merge (squash) May 14, 2025 14:39
@jvela018
jvela018 merged commit e5917db into main May 14, 2025
@jvela018
jvela018 deleted the fix/set_non_linear branch May 14, 2025 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants