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.

Sign up for GitHub

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

Jump to bottom

Ejemyr #20

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Ejemyr #20

wants to merge 23 commits into from

Conversation

Ejemyr
Copy link
Collaborator

@Ejemyr Ejemyr commented Jan 26, 2020

No description provided.

@Ejemyr Ejemyr requested a review from johanhoffman as a code owner January 26, 2020 22:57
ajlace
ajlace reviewed Feb 2, 2020
View reviewed changes
Copy link
Collaborator

@ajlace ajlace left a comment

Choose a reason for hiding this comment

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

Looks good.

As lab 3 is in this PR as well, this will not be approved until lab 3 is correted as well - to avoid confusion.

@ajlace
Copy link
Collaborator

ajlace commented Feb 12, 2020

Lab 3 looks good! Consider including plots showing the accuracy of the methods at x iterations etc.

Christoffer Ejemyr added 2 commits February 17, 2020 23:57
ajlace
ajlace requested changes Feb 18, 2020
View reviewed changes
Copy link
Collaborator

@ajlace ajlace left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Lab 4: Other than the comment elaborating the theory, the lab looks good.

Edit: To calrify both Approximation in 1D and 2D needs clarification in the theory.

Lab-4/ejemyr_lab4.ipynb Show resolved Hide resolved
ajlace
ajlace requested changes Feb 25, 2020
View reviewed changes
Copy link
Collaborator

@ajlace ajlace left a comment

Choose a reason for hiding this comment

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

Lab 5: Except for the comments below, the lab looks great!

Lab-5/ejemyr_lab5.ipynb Outdated Show resolved Hide resolved
Lab-5/ejemyr_lab5.ipynb Show resolved Hide resolved
Christoffer Ejemyr added 3 commits March 2, 2020 22:45
@ajlace
Copy link
Collaborator

ajlace commented Mar 10, 2020

Lab 6 looks good.

@Ejemyr I want to remind you about the required changes for Lab 4 and 5.

@ajlace
Copy link
Collaborator

ajlace commented Mar 10, 2020

Lab 1 and Lab 7 also look good!

@ajlace
Copy link
Collaborator

ajlace commented Apr 28, 2020

Now everything looks good! Great job :)

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

@ajlace ajlace ajlace approved these changes

@johanhoffman johanhoffman Awaiting requested review from johanhoffman johanhoffman is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Ejemyr @ajlace