33 prolongation and restriction tests #34

CodingAllan · 2023-07-06T08:59:46Z

Merge Request - GuideLine Checklist

Guideline to check code before resolve WIP and approval, respectively.
As many checkboxes as possible should be ticked.

Checks by code author:

Always to be checked:

There is at least one issue associated with the pull request.
New code adheres with the coding guidelines
No large data files have been added to the repository. Maximum size for files should be of the order of KB not MB. In particular avoid adding of pdf, word, or other files that cannot be change-tracked correctly by git.

If functions were changed or functionality was added:

Tests for new functionality has been added
A local test was succesful

If new functionality was added:

There is appropriate documentation of your work. (use doxygen style comments)

If new third party software is used:

Did you pay attention to its license? Please remember to add it to the wiki after successful merging.

If new mathematical methods or epidemiological terms are used:

Are new methods referenced? Did you provide further documentation?
The following questions are addressed in the documentation (if need be):
** Developers (what did you do?, how can it be maintained?)
** For users (how to use your work?)
** For admins (how to install and configure your work?)
For documentation: Please write or update the Readme in the current working directory!

Checks by code reviewer(s):

Is the code clean of development artifacts e.g., unnecessary comments, prints, ...
The ticket goals for each associated issue are reached or problems are clearly addressed (i.e., a new issue was introduced).
There are appropriate unit tests and they pass.
The git history is clean and linearized for the merge request. All reviewers should squash commits and write a simple and meaningful commit message.
Coverage report for new code is acceptable.
No large data files have been added to the repository. Maximum size for files should be of the order of KB not MB. In particular avoid adding of pdf, word, or other files that cannot be change-tracked correctly by git.

mknaranja

Hi @CodingAllan, I had a brief look. Generally this looks quite complete but there are some things that could be addressed before a more detailed review. Could you add some comments and also look at what I annotated elsewise?

mknaranja · 2023-07-17T09:16:34Z

tests/test_restriction.cpp

+        gyro::icntl[Param::verbose]        = 0;
+        gyro::icntl[Param::debug]          = 0;
+        gyro::icntl[Param::extrapolation]  = 0;
+        gyro::icntl[Param::DirBC_Interior] = 1;


I think we should test with and without Dirichlet conditions

I can do this, but should the boundary conditions affect the operator ?
I thought that if we prolongate a solution u, then the information on boundary conditions should be stored in the stiffness matrix and right-hand side not neccessarily in the prolongation and restriction operator.

No they shouldnt. And it should be ensured that we have the same operator.

mknaranja · 2023-07-17T09:16:55Z

tests/test_prolongation.cpp

+        gyro::icntl[Param::verbose]        = 0;
+        gyro::icntl[Param::debug]          = 0;
+        gyro::icntl[Param::extrapolation]  = 0;
+        gyro::icntl[Param::DirBC_Interior] = 1;


I think we should test with and without Dirichlet conditions

Additionally (and in contrast) to my comment about "why do you set it if you don't use it?", we should maybe check with parameters that could make a change as in a future version, these might be implemented here. In particular Dirichlet boundary conditions could lead to a simplification but the operator should still handle both cases correctly. However, some parameters are probably never used in the context of the prolongation (also applies to other functions below)

tests/test_restriction.cpp

tests/test_prolongation.cpp

tests/test_restriction.cpp

tests/test_prolongation.cpp

…undary condition. Removed parameters not needed for the test. Soon I will consider to mock functions creating the grid.

… the same ansatz does not work.Look into it next week

codecov · 2023-09-01T09:44:39Z

Codecov Report

❗ No coverage uploaded for pull request base (main@25cd29f). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #34   +/-   ##
=======================================
  Coverage        ?   35.91%           
=======================================
  Files           ?       86           
  Lines           ?     7028           
  Branches        ?        0           
=======================================
  Hits            ?     2524           
  Misses          ?     4504           
  Partials        ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CodingAllan · 2023-09-07T09:24:06Z

Hi @mknaranja , I just wanted to ask if I should add more comments to the program. Aside from that I think that this is ready for review!

mknaranja · 2023-11-23T08:37:53Z

tests/mockgrid.h

+#include "gmgpolar.h"
+
+#ifndef MOCKGRID_H
+    #define MOCKGRID_H


Suggested change

#define MOCKGRID_H

#define MOCKGRID_H

mknaranja · 2024-03-28T13:42:33Z

tests/mockgrid.cpp

+    for (int i = 0; i < new_level->nr; i++) {
+        new_level->r[i] = gyro::dcntl[Param::R0] +
+                          i * (gyro::dcntl[Param::R] - gyro::dcntl[Param::R0]) / new_level->nr; //uniform grid
+    }
+
+    new_level->r[new_level->nr] = gyro::dcntl[Param::R];


Suggested change

for (int i = 0; i < new_level->nr; i++) {

new_level->r[i] = gyro::dcntl[Param::R0] +

i * (gyro::dcntl[Param::R] - gyro::dcntl[Param::R0]) / new_level->nr; //uniform grid

}

new_level->r[new_level->nr] = gyro::dcntl[Param::R];

for (int i = 0; i <= new_level->nr; i++) {

new_level->r[i] = gyro::dcntl[Param::R0] +

static_cast<double>(i) * (gyro::dcntl[Param::R] - gyro::dcntl[Param::R0]) / static_cast<double>(new_level->nr); //uniform grid

}

mknaranja · 2024-03-28T13:54:37Z

tests/mockgrid.cpp

+    new_level->theta = std::vector<double>(new_level->ntheta);
+
+    for (int i = 0; i < new_level->ntheta; i++) {
+        new_level->theta[i] = 2 * PI * i / ntmp; //uniform in theta


Suggested change

new_level->theta[i] = 2 * PI * i / ntmp; //uniform in theta

new_level->theta[i] = 2. * PI * static_cast<double>(i) / ntmp; //uniform in theta

mknaranja · 2024-04-05T19:16:31Z

tests/test_prolongation.cpp

+                EXPECT_EQ(u_test[(j / 2) * ctheta_int + (i / 2)], sol[j * p_level.ntheta_int + i])
+                    << "coarse node injection is failing";
+            }
+            else if (i % 2 != 0 && j % 2 == 0) { // theta_i is fine node


Suggested change

else if (i % 2 != 0 && j % 2 == 0) { // theta_i is fine node

else if (i % 2 != 0 && j % 2 == 0) {

// as numbering in angle (theta_i) is odd, we have a fine node with

// coarse neighbors at (r_i, theta_j - k_{j-1}) and (r_i, theta_j + k_j)

Please adapt comment as following. theta_i itself cannot be coarse or fine. It is one coordinate of the node.

i and j have to be exchanged

mknaranja · 2024-04-05T19:16:47Z

tests/test_prolongation.cpp

+                int i1 = (j / 2) * ctheta_int + (i - 1) / 2; //bottom coarse node in theta
+                int i2 = (i < p_level.ntheta_int - 1) ? i1 + 1 : (j / 2) * ctheta_int; //top coarse node in theta
+
+                double val = (k_q * u_test[i1] + k_qm1 * u_test[i2]);


let's discuss here (stopped here)

CodingAllan added 3 commits July 6, 2023 10:34

prolongation test

0646d89

restriction test

a837ea6

Cmake and restriction

29b959c

CodingAllan requested a review from mknaranja July 6, 2023 08:59

CodingAllan linked an issue Jul 6, 2023 that may be closed by this pull request

Unit tests for prolongation and restriction operators #33

Open

CodingAllan added 3 commits July 6, 2023 14:19

small changes

1ad98fe

small changes for checkbars

53a3106

small changes

3217395

mknaranja requested changes Jul 17, 2023

View reviewed changes

mknaranja mentioned this pull request Jul 17, 2023

Mocking grid creation in prolongation and restriction tests #35

Open

CodingAllan added 3 commits July 21, 2023 16:38

added more detailled description. UNFINSIHED

ae0d85a

added detailled description and testing with and without Dirichlet bo…

8b6365a

…undary condition. Removed parameters not needed for the test. Soon I will consider to mock functions creating the grid.

mocked grid-creating functions in prolongation.cpp in restriction.cpp…

4e38c8d

… the same ansatz does not work.Look into it next week

CodingAllan added 2 commits September 7, 2023 10:32

mock grid functions for restriction.cpp complete

7c8a3d5

remove redundancy

e0826a9

CodingAllan added 3 commits September 11, 2023 10:52

remove further redundancy

8893cde

removing further redundancy

7592254

removing further redundancy

6792af9

CodingAllan requested a review from mknaranja November 23, 2023 08:13

mknaranja reviewed Apr 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

33 prolongation and restriction tests #34

33 prolongation and restriction tests #34

CodingAllan commented Jul 6, 2023 •

edited

mknaranja left a comment

mknaranja Jul 17, 2023

CodingAllan Jul 21, 2023

mknaranja Jul 21, 2023

mknaranja Jul 17, 2023

mknaranja Jul 17, 2023

codecov bot commented Sep 1, 2023 •

edited

CodingAllan commented Sep 7, 2023

mknaranja Nov 23, 2023

mknaranja Mar 28, 2024

mknaranja Mar 28, 2024

mknaranja Apr 5, 2024

mknaranja Apr 18, 2024

mknaranja Apr 5, 2024

	new_level->theta[i] = 2 * PI * i / ntmp; //uniform in theta
	new_level->theta[i] = 2. * PI * static_cast<double>(i) / ntmp; //uniform in theta

33 prolongation and restriction tests #34

Are you sure you want to change the base?

33 prolongation and restriction tests #34

Conversation

CodingAllan commented Jul 6, 2023 • edited

Merge Request - GuideLine Checklist

Checks by code author:

Checks by code reviewer(s):

mknaranja left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 1, 2023 • edited

Codecov Report

CodingAllan commented Sep 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CodingAllan commented Jul 6, 2023 •

edited

codecov bot commented Sep 1, 2023 •

edited