Skip to content

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

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

Merged
merged 8 commits into from
May 14, 2025

Conversation

jvela018
Copy link
Contributor

Issue linked

#6154

@ansys-reviewer-bot
Copy link
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
Copy link
Member

@Samuelopez-ansys Samuelopez-ansys left a comment

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
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
Copy link

codecov bot commented May 14, 2025

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.

Copy link
Collaborator

@MaxJPRey MaxJPRey left a comment

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
34 checks passed
@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