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

[Core] Introduce PointerComparor and remove DofPointerComparor #12509

Merged
merged 4 commits into from
Oct 24, 2024

Conversation

sunethwarna
Copy link
Member

📝 Description
This PR removes the redundant DofPointerComparor because, the operators for Dof<double> is nicely done in the dof.h. This PR introduces more general PointerComparor, which uses the TDataType::operator== to compare. So, if someone was using (atleast I did not find any use cases in the master) the DofPointerComparor, they can easily change and use PointerComparor. The test was changed accordignly.

🆕 Changelog

  • Removes DofPointerComparor
  • Introduces more general PointerComparor
  • Adapts the tests

@sunethwarna sunethwarna requested a review from a team as a code owner July 5, 2024 14:49
@sunethwarna sunethwarna requested review from loumalouomega, a team, pooyan-dadvand, RiccardoRossi, roigcarlo and rubenzorrilla and removed request for a team July 5, 2024 14:49
@sunethwarna
Copy link
Member Author

The failure in DamApplication is unrelated to this. It is failing because, one of the variables which is being used is not included in the solution step variables list. I am not sure who to tag. Any ideas @roigcarlo ?

@roigcarlo
Copy link
Member

On it.

@pooyan-dadvand
Copy link
Member

The implementation is ok for me. Waiting for the unit tests to approve

@sunethwarna sunethwarna merged commit 8a3ec0e into master Oct 24, 2024
9 of 11 checks passed
@sunethwarna sunethwarna deleted the core/key_hash/update_dof_pointer_comparor branch October 24, 2024 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants