Skip to content
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

FEAT: Enable harmonic force #4848

Closed
wants to merge 7 commits into from
Closed

Conversation

JuliusSaitz
Copy link

No description provided.

@JuliusSaitz JuliusSaitz self-assigned this Jun 25, 2024
@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

@JuliusSaitz JuliusSaitz linked an issue Jun 25, 2024 that may be closed by this pull request
2 tasks
Copy link

codecov bot commented Jun 25, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 72.64%. Comparing base (a6445bb) to head (84054f4).
Report is 128 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4848       +/-   ##
===========================================
- Coverage   83.46%   72.64%   -10.82%     
===========================================
  Files         120      120               
  Lines       54562    54562               
===========================================
- Hits        45540    39638     -5902     
- Misses       9022    14924     +5902     

@SMoraisAnsys
Copy link
Collaborator

@JuliusSaitz Thanks a lot for opening this PR. We haven't had the time to work on #4779 so it's really great that you are dealing with it. We are working on refactoring our codebase to deprecate some argument names so this PR will have to follow this and keep use of assignment instead of object.

Pinging @Samuelopez-ansys and @gmalinve for feedback on the changes

@SMoraisAnsys
Copy link
Collaborator

I took the liberty to "commit" by merging the main branch. Doing so will allow your PR to run the tests correctly.

@gmalinve
Copy link
Contributor

@JuliusSaitz good job! thanks for your contribution!
Now you should add some UT to cover the new functionalities you implemented :)

@SMoraisAnsys SMoraisAnsys changed the title Issue 4779 enable harmonic force FEAT: Enable harmonic force Jun 27, 2024
@SMoraisAnsys
Copy link
Collaborator

@JuliusSaitz Could you please add some tests to your PR ? We enforce new development to be tested to avoid adding failing code to the main branch and also to avoid adding bugs in the future.

If needed we can help you with the argument renaming afterward.

@JuliusSaitz
Copy link
Author

JuliusSaitz commented Jul 2, 2024 via email

@SMoraisAnsys
Copy link
Collaborator

Awesome, just wanted to know if you had the will and time to continue the changes :)

@SMoraisAnsys
Copy link
Collaborator

Hi @JuliusSaitz, do you think you'll have time in the comings days/weeks to add the missing tests ? Thanks again for your contribution !

@JuliusSaitz
Copy link
Author

JuliusSaitz commented Aug 19, 2024 via email

@Samuelopez-ansys
Copy link
Member

Hi @JuliusSaitz ,

What is the status of this PR? Please could you finish it? If not we will close the PR without merging.

Thanks!

@JuliusSaitz
Copy link
Author

JuliusSaitz commented Oct 2, 2024 via email

@gmalinve gmalinve closed this Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

m2d.enable_harmonic_force does not have a complete set of inputs
4 participants