kyriv1980 / Dummy

0 stars 0 forks source link

v_kyr1ak/refactoring [!35 merged] #108

Closed kyriv1980 closed 2 months ago

kyriv1980 commented 2 months ago

Imported from https://github.inl.gov/ncrc/subchannel/issues/108 : @moose-ncrc created issue at Feb 04, 2021 02:22PM MST:

In GitLab by @kyriv1980 on Feb 4, 2021, 14:22

_Merges vkyr1ak/refactoring -> devel

This merge will resolve issue #13

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 04, 2021 03:05PM MST:

In GitLab by @moosetest on Feb 4, 2021, 15:05

Job Test on 6b4c27a wanted to post the following:

Code coverage: deleted

This comment will be updated on new commits.

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 04, 2021 03:05PM MST:

In GitLab by @moosetest on Feb 4, 2021, 15:05

CIVET Testing summary for 0bfac45

Precheck : Passed
Test : Passed
Test dbg : Passed
Non unity build : Passed
App tests : Passed

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 05, 2021 11:08AM MST:

In GitLab by @andrsd on Feb 5, 2021, 11:08

not beautiful :-)

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 05, 2021 11:08AM MST:

In GitLab by @andrsd on Feb 5, 2021, 11:08

I don't understand this error message. If I get it as a use, what I am supposed to do with it?

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 05, 2021 11:08AM MST:

In GitLab by @andrsd on Feb 5, 2021, 11:08

Probably a left over debug statement?

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 05, 2021 11:09AM MST:

In GitLab by @andrsd on Feb 5, 2021, 11:09

Your error message are in general confusing. If you are using them, they need to be helpful to the user.

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 06, 2021 10:13AM MST:

In GitLab by @kyriv1980 on Feb 6, 2021, 10:13

You are right. I had this comment to remind me that I need to edit in the future but now I deleted it.

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 06, 2021 10:20AM MST:

In GitLab by @kyriv1980 on Feb 6, 2021, 10:20

If I have 10 levels I have 11 nodes. But at node zero is the inlet and node 10 is the outlet. Boundary conditions live there. So some calculations can't happen on these nodes. I know the nodalization so I wouldn't make that mistake... but in the future someone who would want to use these functions in a different algorithm might make that mistake. I just wanted to get ahead of that.

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 06, 2021 10:22AM MST:

In GitLab by @kyriv1980 on Feb 6, 2021, 10:22

Just a note explaining how I take the average linear power rate for that cell. I will delete it.

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 06, 2021 10:25AM MST:

In GitLab by @kyriv1980 on Feb 6, 2021, 10:25

changed this line in version 2 of the diff

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 06, 2021 10:25AM MST:

In GitLab by @kyriv1980 on Feb 6, 2021, 10:25

added 1 commit

Compare with previous version

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 06, 2021 11:04AM MST:

In GitLab by @moosetest on Feb 6, 2021, 11:04

CIVET Testing summary for 06a3795

Precheck : Passed
Test : Passed
Test dbg : Passed
Non unity build : Passed
App tests : Passed

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 08, 2021 05:50AM MST:

In GitLab by @andrsd on Feb 8, 2021, 05:50

The error message is better now, but I think what you call node is actually axial level. Is it not?

I wanted to suggest this, for example:

": Cannot compute crossflow at axial level 0. Boundary condition should be applied here instead."

but, you were faster changing it. Note that the : is important - it separates the name from the error message.

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 08, 2021 10:35AM MST:

In GitLab by @kyriv1980 on Feb 8, 2021, 10:35

Axial level is connected to the cell that has a height dz. The nodes in this case are the inlet and the outlet of the cell. So node 0 is the inlet of the assembly where boundary conditions live. Because I do not use a staggered mesh for pressure and mass flow the average quantities I calculate for each cell are placed at either the inlet node or the outlet node for each cell. Because I assume only positive flow, information flows from inlet to outlet. So some quantities of the average cell values are placed on the inlet node.

In the future when I edit the equations to deal with negative mass flow I will have to edit this concept. Whoever wants to make a code that wants to deal with negative mass flow will have to derive the discretized equations again from scratch.

The resulting equations will not be much different from what we have now, but they will be different.

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 08, 2021 10:43AM MST:

In GitLab by @kyriv1980 on Feb 8, 2021, 10:43

changed this line in version 3 of the diff

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 08, 2021 10:43AM MST:

In GitLab by @kyriv1980 on Feb 8, 2021, 10:43

resolved all threads

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 08, 2021 10:43AM MST:

In GitLab by @kyriv1980 on Feb 8, 2021, 10:43

added 1 commit

Compare with previous version

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 08, 2021 11:23AM MST:

In GitLab by @moosetest on Feb 8, 2021, 11:23

CIVET Testing summary for 6b4c27a

Precheck : Passed
Test : Passed
Test dbg : Passed
Non unity build : Passed
App tests : Passed

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 08, 2021 12:18PM MST:

In GitLab by @andrsd on Feb 8, 2021, 12:18

Ok, I though that axial level was the level with nodes, so I just need to remember this.

My point was this: Nodes are globally numbered, so inlet condition in the following "image" is applied at node 0 and 3. So telling used about node 0 is confusing.

 5    2
 |    |
 4    1
 |    |
 3    0

Anyway, thanks for the other edit, I think the error message is good now.

kyriv1980 commented 2 months ago

@moose-ncrc commented at Feb 08, 2021 12:19PM MST:

In GitLab by @andrsd on Feb 8, 2021, 12:19

mentioned in commit 0459c8658a07c43a2d1828037d07217c33f98414

kyriv1980 commented 2 months ago

Issue closed at: Jan 16, 2023 07:04PM MST by @moose-ncrc