-
Notifications
You must be signed in to change notification settings - Fork 14
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
synchronizeMaterialInCells implementation using Accelerator API #1584
synchronizeMaterialInCells implementation using Accelerator API #1584
Conversation
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.
Please do the following actions:
- re-indent all the source files using clang-format to make the indentation coherent.
- add tests for your implementation
@@ -0,0 +1,136 @@ | |||
#ifndef ACCELERATOR_INDEX_SELECTER_H |
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.
- Add correct header.
- This file should not be here because it uses Arcane Internal API and so it can not be used outside of Arcane. A better place should be in materials/internal.
- Do not use
IParallelMng
in this class. Instead pass a RunQueue. - Use only needed header to compile this file (for example no need to include
arcane/SubDomain.h
) - Use full name for class member name (for example replace
m_is_acc_avl
bym_is_accelerator_policy
)
@@ -240,7 +240,7 @@ class ParallelMngDispatcher::Impl | |||
{ | |||
if (m_is_accelerator_aware_disabled) | |||
return false; | |||
if (!m_queue.get()) | |||
if (!m_queue.get() || m_runner->executionPolicy() == Arcane::Accelerator::eExecutionPolicy::Sequential) |
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.
Do not modify it. Add the test to the user of this class
#ifdef ARCANE_COMPONENT_arcane_materials | ||
/*#ifdef ARCANE_COMPONENT_arcane_materials | ||
#ifndef ARCANE_TRACE_ENUMERATOR | ||
#define ARCANE_TRACE_ENUMERATOR | ||
#endif | ||
#endif | ||
#endif*/ |
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.
Why this modification ? Please restore the original version.
@@ -140,6 +140,7 @@ MeshMaterialMng(const MeshHandle& mesh_handle,const String& name) | |||
String s = platform::getEnvironmentVariable("ARCANE_ALLENVCELL_FOR_RUNCOMMAND"); | |||
if (!s.null()) | |||
m_is_allcell_2_allenvcell = true; | |||
m_mms = new MeshMaterialSynchronizer(this); |
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.
Use full name for member variable (m_material_synchronzizer
)
#include "arcane/utils/TraceAccessor.h" | ||
#include "arcane/utils/ArrayView.h" | ||
|
||
#include "arcane/VariableTypedef.h" | ||
|
||
#include "arcane/materials/MaterialsGlobal.h" | ||
#include "arcane/materials/MatItem.h" | ||
|
||
#include "arcane/materials/CellToAllEnvCellConverter.h" | ||
#include "arcane/accelerator/core/RunQueue.h" | ||
|
||
#include "arcane/accelerator/Accelerator.h" | ||
#include "arcane/accelerator/IndexSelecter.h" | ||
#include "arcane/accelerator/SpanViews.h" | ||
#include "arcane/IApplication.h" | ||
#include "arcane/core/ISubDomain.h" | ||
#include "arcane/accelerator/Reduce.h" | ||
#include "arcane/accelerator/Runner.h" | ||
#include "arcane/accelerator/VariableViews.h" | ||
#include "arcane/accelerator/Accelerator.h" | ||
#include "arcane/accelerator/RunCommandLoop.h" | ||
#include "arcane/accelerator/RunCommandEnumerate.h" | ||
#include "arcane/accelerator/core/RunQueueBuildInfo.h" | ||
#include "arcane/accelerator/core/Memory.h" | ||
#include "arcane/accelerator/MaterialVariableViews.h" | ||
#include "arcane/accelerator/RunCommandMaterialEnumerate.h" | ||
#include "arcane/accelerator/core/IAcceleratorMng.h" | ||
#include "arcane/accelerator/core/RunQueueEvent.h" |
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.
Remove unneeded headers
#include "arcane/utils/TraceAccessor.h" | ||
#include "arcane/utils/ArrayView.h" | ||
|
||
#include "arcane/VariableTypedef.h" | ||
|
||
#include "arcane/materials/MaterialsGlobal.h" | ||
#include "arcane/materials/MatItem.h" | ||
|
||
#include "arcane/materials/CellToAllEnvCellConverter.h" | ||
#include "arcane/accelerator/core/RunQueue.h" | ||
|
||
#include "arcane/accelerator/Accelerator.h" | ||
#include "arcane/accelerator/IndexSelecter.h" | ||
#include "arcane/accelerator/SpanViews.h" | ||
#include "arcane/IApplication.h" | ||
#include "arcane/core/ISubDomain.h" | ||
#include "arcane/accelerator/Reduce.h" | ||
#include "arcane/accelerator/Runner.h" | ||
#include "arcane/accelerator/VariableViews.h" | ||
#include "arcane/accelerator/Accelerator.h" | ||
#include "arcane/accelerator/RunCommandLoop.h" | ||
#include "arcane/accelerator/RunCommandEnumerate.h" | ||
#include "arcane/accelerator/core/RunQueueBuildInfo.h" | ||
#include "arcane/accelerator/core/Memory.h" | ||
#include "arcane/accelerator/MaterialVariableViews.h" | ||
#include "arcane/accelerator/RunCommandMaterialEnumerate.h" | ||
#include "arcane/accelerator/core/IAcceleratorMng.h" | ||
#include "arcane/accelerator/core/RunQueueEvent.h" | ||
|
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.
Remove unneeded header
#include "arcane/utils/TraceAccessor.h" | ||
#include "arcane/utils/ArrayView.h" | ||
|
||
#include "arcane/VariableTypedef.h" | ||
#include <arcane/core/MeshVariableArrayRef.h> | ||
|
||
#include "arcane/materials/MaterialsGlobal.h" | ||
#include "arcane/materials/MatItem.h" | ||
|
||
#include "arcane/materials/CellToAllEnvCellConverter.h" | ||
#include "arcane/accelerator/core/RunQueue.h" | ||
|
||
#include "arcane/accelerator/Accelerator.h" | ||
#include "arcane/accelerator/IndexSelecter.h" | ||
#include "arcane/accelerator/SpanViews.h" | ||
#include "arcane/IApplication.h" | ||
#include "arcane/core/ISubDomain.h" | ||
#include "arcane/accelerator/Reduce.h" | ||
#include "arcane/accelerator/Runner.h" | ||
#include "arcane/accelerator/VariableViews.h" | ||
#include "arcane/accelerator/Accelerator.h" | ||
#include "arcane/accelerator/RunCommandLoop.h" | ||
#include "arcane/accelerator/RunCommandEnumerate.h" | ||
#include "arcane/accelerator/core/RunQueueBuildInfo.h" | ||
#include "arcane/accelerator/core/Memory.h" | ||
#include "arcane/accelerator/MaterialVariableViews.h" | ||
#include "arcane/accelerator/RunCommandMaterialEnumerate.h" | ||
#include "arcane/accelerator/core/IAcceleratorMng.h" | ||
#include "arcane/accelerator/core/RunQueueEvent.h" | ||
|
||
#include "arcane/materials/internal/IMeshMaterialSynchronizerImpl.h" |
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.
Remove unneeded headers
#include "arcane/materials/CellToAllEnvCellConverter.h" | ||
#include "arcane/accelerator/core/RunQueue.h" | ||
|
||
#include "arcane/accelerator/Accelerator.h" | ||
#include "arcane/accelerator/IndexSelecter.h" | ||
#include "arcane/accelerator/SpanViews.h" | ||
#include "arcane/IApplication.h" | ||
#include "arcane/core/ISubDomain.h" | ||
#include "arcane/accelerator/Reduce.h" | ||
#include "arcane/accelerator/Runner.h" | ||
#include "arcane/accelerator/VariableViews.h" | ||
#include "arcane/accelerator/Accelerator.h" | ||
#include "arcane/accelerator/RunCommandLoop.h" | ||
#include "arcane/accelerator/RunCommandEnumerate.h" | ||
#include "arcane/accelerator/core/RunQueueBuildInfo.h" | ||
#include "arcane/accelerator/core/Memory.h" | ||
#include "arcane/accelerator/MaterialVariableViews.h" | ||
#include "arcane/accelerator/RunCommandMaterialEnumerate.h" | ||
#include "arcane/accelerator/core/IAcceleratorMng.h" | ||
#include "arcane/accelerator/core/RunQueueEvent.h" | ||
#include "arcane/utils/ValueConvert.h" | ||
|
||
#include "arcane/materials/internal/IMeshMaterialSynchronizerImpl.h" | ||
#include "arcane/materials/internal/MeshMaterialSynchronizerImplAcc.h" | ||
#include "arcane/materials/internal/MeshMaterialSynchronizerImpl.h" |
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.
Remove unneeded headers
MeshMaterialSynchronizerImpl.cc | ||
MeshMaterialSynchronizerImplAcc.cc |
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.
Please rename MeshMaterialSynchronizerImpl
to LegacyMeshMaterialSynchronizerImpl
and MeshMaterialSynchronizerImplAcc
to AcceleratorMeshMaterialSynchronizerImpl
.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1584 +/- ##
==========================================
+ Coverage 69.60% 69.63% +0.03%
==========================================
Files 2241 2245 +4
Lines 160221 160357 +136
Branches 18470 18484 +14
==========================================
+ Hits 111514 111659 +145
+ Misses 42052 42042 -10
- Partials 6655 6656 +1 ☔ View full report in Codecov by Sentry. |
At line https://github.com/reynierf/framework/blob/a8e44d0fcb60638575f28a5e5a4a02b7661e825d/arcane/src/arcane/materials/MeshMaterialSynchronizerImplAcc.cc#L41 , could we allocate , m_mat_presence(VariableBuildInfo(material_mng->mesh(),"ArcaneMaterialSyncPresence", IVariable::PTemporary|IVariable::PSubDomainDepend)) It would prevent positive false when we compare mono-domain vs. multi-domain. |
0df7d6d
to
eed1c97
Compare
This is an implementation of synchronizeMaterialInCells using Accelerator API.
Can be enabled by setting
ARCANE_ACC_MAT_SYNCHRONIZER=1
, using original version otherwise.