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

Taskflow workstream review #21

Closed
wants to merge 492 commits into from
Closed

Conversation

tjhei
Copy link
Owner

@tjhei tjhei commented Jun 11, 2024

No description provided.

@tjhei tjhei changed the title Taskflow workstream Taskflow workstream review Jun 11, 2024
@@ -661,7 +661,140 @@ namespace WorkStream

} // namespace tbb_no_coloring
# endif // DEAL_II_WITH_TBB
# ifdef DEAL_II_WITH_TASKFLOW
Copy link
Owner Author

Choose a reason for hiding this comment

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

3 empty lines before

include/deal.II/base/work_stream.h Outdated Show resolved Hide resolved
typename Iterator,
typename ScratchData,
typename CopyData>
void run(const Iterator & begin,
Copy link
Owner Author

Choose a reason for hiding this comment

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

can you add a comment to the run function as

/**
*
*/

that mentions that you are ignoring the last 2 arguments?


unsigned int idx = 0;

std::vector<std::unique_ptr<CopyData>> copy_datas;
Copy link
Owner Author

Choose a reason for hiding this comment

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

add a comment that this is used to communicate between worked and copier?

include/deal.II/base/work_stream.h Outdated Show resolved Hide resolved

// Find our currently used scratch data and
// mark it as unused.
for (typename ScratchDataList::iterator p =
Copy link
Owner Author

Choose a reason for hiding this comment

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

I think this could be a ranged-for:

Suggested change
for (typename ScratchDataList::iterator p =
for (auto &p : scratch_data_list)

@RyanMoulday RyanMoulday force-pushed the Taskflow-Workstream branch 5 times, most recently from d05c8e0 to ecc709c Compare June 14, 2024 03:39
@RyanMoulday RyanMoulday force-pushed the Taskflow-Workstream branch from ecc709c to 79d0fc7 Compare June 21, 2024 18:11
kronbichler and others added 22 commits July 11, 2024 16:56
step-87: Clear ghost values in a vector
Add DiscreteTime::serialize() and DiscreteTime::memory_consumption()
CMake: Explain why we disable dynamic linking on Windows.
step-29 no longer uses deallog. Update output in results.dox accordingly.
Updates to the LogStream documentation.
Remove an unnecessary 'typename' in step-19.
Make a 'Mapping' object 'const'.
Support codim-1 in hyper_shell() and hyper_cube_with_cylindrical_hole()
…_index_nodal_simplex

Add `face_to_cell_index_nodal` for simplex elements
bangerth and others added 27 commits July 23, 2024 16:25
.gitattributes: update for testsuite changes
MGTwoLevelTransfer: Use function without constraints forgotten before
Fix MSVC not finding an overloaded function.
FEFaceEvaluation: Implement normal hessian
Windows: Use GNU patch instead of Strawberry Perl patch.
Post release branch: Require the current version of deal.II
Post release branch: switch DEAL_II_DEPRECATED_EARLY to DEAL_II_DEPRECATED
Obey the rule of 5 or 7 in TaskResult.
@RyanMoulday RyanMoulday force-pushed the Taskflow-Workstream branch from e7144c0 to e14716c Compare July 25, 2024 00:51
@tjhei tjhei closed this Sep 2, 2024
@RyanMoulday RyanMoulday deleted the Taskflow-Workstream branch September 7, 2024 21:10
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.