-
Notifications
You must be signed in to change notification settings - Fork 159
DOCS: improving documentation of Maxwell class #6150
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
base: main
Are you sure you want to change the base?
Conversation
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
@anur7 Please could you be more specific in the PR Title? It is important because if you are going to change only one method, then I would change the title to "Improve xxxx method documentation" And then please add some description. Thank you! |
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #6150 +/- ##
===========================================
- Coverage 38.69% 18.64% -20.06%
===========================================
Files 171 171
Lines 64941 64941
===========================================
- Hits 25131 12109 -13022
- Misses 39810 52832 +13022 🚀 New features to boost your workflow:
|
# Conflicts: # src/ansys/aedt/core/maxwell.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, Abdun! Thank you.
src/ansys/aedt/core/maxwell.py
Outdated
@@ -529,6 +561,7 @@ def setup_ctrlprog( | |||
def eddy_effects_on(self, assignment, enable_eddy_effects=True, enable_displacement_current=True): | |||
"""Assign eddy effects on a list of objects. | |||
|
|||
Available only for Eddy Current and Transient solvers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 3D AC Magnetic A-Phi solver does not support eddy effects off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nunesanderson, @gmalinve AC Magnetic A-Phi solver is still beta that's why I did not mention it.
>>> setup.props["TimeStep"] = "1/500s" | ||
>>> m3d.analyze(setup=setup.name, use_auto_settings=False) | ||
>>> m3d.export_element_based_harmonic_force() | ||
>>> m3d.release_desktop(True, True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about mentioning about the what the output of this method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is clear reading the above description. As written the output is a .csv file containing element based harmonic force data
>>> m3d = Maxwell3d(solution_type="Magnetostatic") | ||
>>> cylinder = m3d.modeler.create_cylinder(origin=[0, 0, 0], radius=5, height=15, orientation="Z") | ||
>>> m3d.assign_voltage_drop(assignment=cylinder.top_face_z, amplitude="1V", name="Volt", swap_direction=False) | ||
>>> m3d.release_desktop(True, True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the excitation set in such a way the model would work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nunesanderson here just showing in a generic way how you can use the method to assign the voltage drop excitation
src/ansys/aedt/core/maxwell.py
Outdated
@@ -1934,6 +2133,9 @@ def export_element_based_harmonic_force( | |||
): | |||
"""Export an element-based harmonic force data to a .csv file. | |||
|
|||
To apply this method, first it is needed to enable element based harmonic force, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the user need to run first and then run the script to apply this method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nunesanderson here I meant before using this method it is needed to enable element based harmonic force and simulate the model. If you look at the example below, first I enabled the element based harmonic force, run the model, and after used the method to export element based harmonic force.
Description
Adding examples and improving docstring of Maxwell's methods.
Issue linked
#6136
Checklist