Skip to content

Conversation

@loumalouomega
Copy link
Member

@loumalouomega loumalouomega commented Jun 4, 2025

📝 Description

This PR replaces direct OpenMP pragmas in kratos/spaces/ublas_space.h with Kratos ParallelUtilities (primarily IndexPartition).

This change modernizes the shared-memory parallelism approach in UblasSpace, aligning it with the recommended Kratos API.

The following functions were refactored:

  • Copy (VectorType)
  • Dot
  • TwoNorm (Matrix and compressed_matrix)
  • JacobiNorm (Matrix)
  • Mult (compressed_matrix, via ParallelProductNoAdd)
  • InplaceMult
  • Assign
  • UnaliasedAdd
  • ResizeData (all three overloads)
  • SetToZero (all three overloads)
  • The private helper ParallelProductNoAdd was significantly refactored to use IndexPartition, and its own helpers CreatePartition and partial_product_no_add were removed. (TODO)

Reductions were handled using Kratos reducer objects like SumReduction.

🆕 Changelog

This PR replaces direct `OpenMP` pragmas in `kratos/spaces/ublas_space.h` with Kratos `ParallelUtilities` (primarily `IndexPartition`).

This change modernizes the shared-memory parallelism approach in UblasSpace, aligning it with the recommended Kratos API.

The following functions were refactored:
- `Copy` (VectorType)
- `Dot`
- `TwoNorm` (`Matrix` and `compressed_matrix`)
- `JacobiNorm` (`Matrix`)
- `Mult` (`compressed_matrix`, via ParallelProductNoAdd)
- `InplaceMult`
- `Assign`
- `UnaliasedAdd`
- `ResizeData` (all three overloads)
- `SetToZero` (all three overloads)
- The private helper `ParallelProductNoAdd` was significantly refactored to use `IndexPartition`, and its own helpers `CreatePartition` and `partial_product_no_add` were removed.

Reductions were handled using Kratos reducer objects like `SumReduction`.
@loumalouomega loumalouomega requested a review from a team as a code owner June 4, 2025 21:40
@loumalouomega loumalouomega added Kratos Core Parallel-SMP Shared memory parallelism with OpenMP or C++ Threads Legacy Old code not maintained Refactor When code is moved or rewrote keeping the same behavior labels Jun 4, 2025
@loumalouomega loumalouomega enabled auto-merge June 4, 2025 21:40
@loumalouomega
Copy link
Member Author

Funny thing is that I think I found several bugs with this. Tests failing are due to incorrect size vectors.

loumalouomega and others added 21 commits June 6, 2025 11:42
…r int, bool, double, array_1d, vector, and Matrix type variables.
Co-authored-by: Suneth Warnakulasuriya <7856520+sunethwarna@users.noreply.github.com>
Changes include:
- The UDSM-based constitutive laws are no longer copy-able. Use member function `Clone` for creating deep copies. These constitutive laws are still not moveable. Compile-time checks have been added to enforce this part of the design.
- By introducing a data member of type `ConstitutiveLawDimension`, we have been able to generalize class `SmallStrainUDSM3DLaw` such that class `SmallStrainUDSM2DPlaneStrainLaw` became redundant. The new data member is used for getting the strain size as well as the local space dimension. Consequently, we could get rid of static data members `VoigtSize` and `Dimension`.
- Class `SmallStrainUDSM3DLaw` has been renamed to `SmallStrainUDSMLaw`. The corresponding files have been renamed, too. Class `SmallStrainUDSM2DPlaneStrainLaw` has been removed.
- Moved function definitions from header files to the corresponding implementation files.
- Removed a few `override`s that were identical to the base class implementation.
- Removed many redundant comments.
- Removed several redundant name qualifiers.
- Removed several unused member functions.
- Corrected the implementations of `save` and `load`, and made them more complete.
- Removed several `KRATOS_TRY` and `KRATOS_CATCH` constructs, to avoid catching errors in places where they can't be handled properly anyway.
- Made the implementation of member `Clone` complete (several data members were not handled).
- Use square brackets (i.e. `[]`) rather than parentheses (i.e. `()`) to access vector entries. This is in line with how the STL accesses elements of containers.
- Renamed function parameters `rThisVariable` to `rVariable`.
@loumalouomega loumalouomega requested a review from a team as a code owner June 9, 2025 07:50
Copy link
Member

@RiccardoRossi RiccardoRossi left a comment

Choose a reason for hiding this comment

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

minor comments. ignore if you dislike them

@loumalouomega
Copy link
Member Author

Should be fine now @RiccardoRossi

…or improved parallelism and performance"

This reverts commit ceacb6f.
@loumalouomega
Copy link
Member Author

Should be fine now @RiccardoRossi

Ping @RiccardoRossi

@loumalouomega
Copy link
Member Author

Should be fine now @RiccardoRossi

Ping @RiccardoRossi

Hello

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Cleanup Kratos Core Legacy Old code not maintained Parallel-SMP Shared memory parallelism with OpenMP or C++ Threads Refactor When code is moved or rewrote keeping the same behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants