Skip to content

Add some explanations for the strategy solving the Poisson problems with non-homogeneous Dirichlet boundary conditions#127

Merged
amartinhuertas merged 8 commits intogridap:masterfrom
wei3li:master
Jan 27, 2022
Merged

Add some explanations for the strategy solving the Poisson problems with non-homogeneous Dirichlet boundary conditions#127
amartinhuertas merged 8 commits intogridap:masterfrom
wei3li:master

Conversation

@wei3li
Copy link
Copy Markdown
Contributor

@wei3li wei3li commented Jan 25, 2022

No description provided.

@amartinhuertas amartinhuertas self-requested a review January 25, 2022 11:29
@amartinhuertas
Copy link
Copy Markdown
Member

amartinhuertas commented Jan 26, 2022

Hi @wei3li ! Thanks for your work. I will start now with my review.

@amartinhuertas
Copy link
Copy Markdown
Member

I have generated the page of the tutorial locally on my machine, and I noted that there are some typos, and line breaks missing in the result. This is in particular what I see:

Screenshot from 2022-01-27 10-41-19

I guess that you modified and pushed the tutorial sources without double checking that the generated page looks as you were expecting, right? No problem. Indeed, I couldnt find the instructions on the README file nor in the documentation of the Tutorials repo in order to generate locally the HTML pages of the tutorials.

To this end, you have to follow the following instructions once:

julia --project=docs   # From the Unix shell, located at the root of Tutorials repo 
develop .              # From the Julia package manager prompt
instantiate            # "" 
build                  # "" 
exit()                 # From the Julia REPL

and then, each time that you perform a change on the tutorial sources, you have to execute the following from the shell:

julia --project=docs docs/make.jl # From the Unix shell, located at the root of Tutorials repo 

to generate the HTML pages. The pages generated are available at "Tutorials/docs/build/pages". You can open them, e.g., with Google Chrome. Perhaps, once you are familiar with these instructions, we may add a section to the README file, something like "Generating tutorial HTML pages locally" or in the Wiki of the repo, https://github.com/gridap/Tutorials/wiki.

As a first step in the review process, can you please modify the sources of the tutorial so that the generated page looks as you were expecting and push your changes? Thanks!

On the other hand, with respect to the contents, while they are correct, they go may be too straight to the point. I think is better to add some more narrative and motivation about why we are doing this. I can do this part once you add your changes, ok?

@amartinhuertas
Copy link
Copy Markdown
Member

amartinhuertas commented Jan 27, 2022

For the records, @wei3li was able to generate locally the ipynb files using Literate.jl, and they looked ok for the changes at hand. The issue seems to be in the generated HTML pages, not actually in the ipynb files.

@amartinhuertas
Copy link
Copy Markdown
Member

Hereby I confirm that, after the changes in 84e6616, the issues with the HTML file have disappeared. Nice work @wei3li ! I will know continue with my review.

Copy link
Copy Markdown
Member

@amartinhuertas amartinhuertas left a comment

Choose a reason for hiding this comment

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

Hi @wei3li, I have finished my review. You can proceed with the requested changes. Thanks!

Comment thread src/poisson_dev_fe.jl Outdated
Comment thread src/poisson_dev_fe.jl Outdated
Comment thread src/poisson_dev_fe.jl
Comment thread src/poisson_dev_fe.jl Outdated
Comment thread src/poisson_dev_fe.jl Outdated
Copy link
Copy Markdown
Member

@amartinhuertas amartinhuertas left a comment

Choose a reason for hiding this comment

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

@wei3li ... thanks for your work! I just added a pair of additional comments. The tutorial is looking much better!

Comment thread src/poisson_dev_fe.jl Outdated
Comment thread src/poisson_dev_fe.jl Outdated
Comment thread src/poisson_dev_fe.jl Outdated
Comment thread src/poisson_dev_fe.jl
Copy link
Copy Markdown
Member

@amartinhuertas amartinhuertas left a comment

Choose a reason for hiding this comment

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

@wei3li ... I am satisfied with the current changes. I will accept the PR now. Thanks!

@amartinhuertas amartinhuertas merged commit 9fe93ed into gridap:master Jan 27, 2022
@wei3li
Copy link
Copy Markdown
Contributor Author

wei3li commented Jan 27, 2022

One more commit, I added the definition to $g_0^h$.

@amartinhuertas
Copy link
Copy Markdown
Member

One more commit, I added the definition to $g_0^h$.

Ok, I already noticed it. The changes are already available at the official tutorials repo.

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.

2 participants