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

Add distributed moving laser heating #92

Merged
merged 15 commits into from
May 18, 2023
Merged

Conversation

MarkMa1990
Copy link
Contributor

This program aims to solve the non-uniform isotropic heat equation with distributed memory.

as suggested by wolfgang, the program is moved here.
dealii/dealii#13023 (comment)

MarkMa1990 and others added 4 commits December 2, 2021 17:51
change dependency
add figures in readme
correct images
Copy link
Member

@peterrum peterrum left a comment

Choose a reason for hiding this comment

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

Sorry for getting to this so later. I left some comments. Looks overall good.

Comment on lines 71 to 72
// this library is deprecated. use affine_constraints.h instead
//#include <deal.II/lac/constraint_matrix.h>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// this library is deprecated. use affine_constraints.h instead
//#include <deal.II/lac/constraint_matrix.h>
Suggested change
// this library is deprecated. use affine_constraints.h instead
//#include <deal.II/lac/constraint_matrix.h>
// this library is deprecated. use affine_constraints.h instead
//#include <deal.II/lac/constraint_matrix.h>

Comment on lines +92 to +97
#ifndef GLOBAL_PARA
#define GLOBAL_PARA
#include "./globalPara.h"
#include "./boundaryInit.h"
#include "./rightHandSide.h"
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Is this ifdef needed?

Comment on lines +92 to +97
#ifndef GLOBAL_PARA
#define GLOBAL_PARA
#include "./globalPara.h"
#include "./boundaryInit.h"
#include "./rightHandSide.h"
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Is this ifdef needed?

@@ -0,0 +1 @@
Laplace equation surface coupled to an external simulation program (here simply a fancy boundary condition) using the coupling library preCICE.
Copy link
Member

Choose a reason for hiding this comment

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

?

Distributed_Moving_Laser_Heating/docs_old/intro.dox Outdated Show resolved Hide resolved
where $n$ is the time step number, and $dt$ is the time step.

<h3> The initial condition </h3>
Deall.II has provide an interpolation function (VectorTools::interpolate) to distribute initial conditions. However, in this tutorial,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Deall.II has provide an interpolation function (VectorTools::interpolate) to distribute initial conditions. However, in this tutorial,
deall.II provides an interpolation function (VectorTools::interpolate) to distribute initial conditions. However, in this tutorial,
Suggested change
Deall.II has provide an interpolation function (VectorTools::interpolate) to distribute initial conditions. However, in this tutorial,
Deall.II has provide an interpolation function (VectorTools::interpolate) to distribute initial conditions. However, in this tutorial,

Distributed_Moving_Laser_Heating/README.md Outdated Show resolved Hide resolved

![illustration](./images/structure-2d.png)

## numerical results
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## numerical results
## Numerical results
Suggested change
## numerical results
## numerical results

@bangerth
Copy link
Member

@MarkMa1990 I will second @peterrum 's observation that we let this sit around for too long without giving you feedback :-(

@peterrum left a number of comments on this program. Do you think you have the time to address them and upload a new version? I will then look at it as well!

@MarkMa1990
Copy link
Contributor Author

MarkMa1990 commented Jul 18, 2022 via email

@bangerth
Copy link
Member

Excellent, we look forward to it!

MarkMa1990 and others added 9 commits July 25, 2022 10:09
Co-authored-by: Peter Munch <peterrmuench@gmail.com>
Co-authored-by: Peter Munch <peterrmuench@gmail.com>
delete the commented petsc part
…ing.cc


accepted

Co-authored-by: Peter Munch <peterrmuench@gmail.com>
…ing.cc

Co-authored-by: Peter Munch <peterrmuench@gmail.com>
…ing.cc

Co-authored-by: Peter Munch <peterrmuench@gmail.com>
@bangerth
Copy link
Member

Oh, did we never notice that you updated all of the things that @peterrum had commented on? In other words, is this ready to be merged from your side?

@bangerth
Copy link
Member

My apologies that none of us saw that! In the future, add a comment whenever you address comments and we know that we should take another look.

Copy link
Member

@marcfehling marcfehling left a comment

Choose a reason for hiding this comment

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

I've made a few changes so that your program compiles and runs with the latest deal.II 9.4.0 release.

I have one question: What is the purpose of the docs_old folder? If it doesn't serve any purpose, I would vote to delete it. There is documentation in that folder though, i.e. intro.dox and results.dox, that we should save and move to docs.

Otherwise I would say that after @peterrum's extensive review this is ready to merge.

@marcfehling marcfehling force-pushed the master branch 3 times, most recently from 549fe5e to 7aef0d6 Compare April 27, 2023 06:07
@marcfehling
Copy link
Member

I did another force push to make sure that this code compiles on the new CI workflow introduced in #142.

Copy link
Member

@peterrum peterrum left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment on lines 43 to 48
/*
#include <deal.II/lac/petsc_parallel_vector.h>
#include <deal.II/lac/petsc_parallel_sparse_matrix.h>
#include <deal.II/lac/petsc_solver.h>
#include <deal.II/lac/petsc_precondition.h>
*/
Copy link
Member

Choose a reason for hiding this comment

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

?

@peterrum peterrum merged commit f2bfb2a into dealii:master May 18, 2023
2 checks passed
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.

None yet

4 participants