Subdomain basic statistics . #2119

aalfonsi · 2023-05-22T19:10:35Z

Pull Request Description

What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)

Closes #1923

What are the significant changes in functionality due to this change request?

Added a new post processor to compute subdomain basic statistics leveraging RAVEN basic statistics.

For Change Control Board: Change Request Review

The following review must be completed by an authorized member of the Change Control Board.

1. Review all computer code.
2. If any changes occur to the input syntax, there must be an accompanying change to the user manual and xsd schema. If the input syntax change deprecates existing input files, a conversion script needs to be added (see Conversion Scripts).
3. Make sure the Python code and commenting standards are respected (camelBack, etc.) - See on the wiki for details.
4. Automated Tests should pass, including run_tests, pylint, manual building and xsd tests. If there are changes to Simulation.py or JobHandler.py the qsub tests must pass.
5. If significant functionality is added, there must be tests added to check this. Tests should cover all possible options. Multiple short tests are preferred over one large test. If new development on the internal JobHandler parallel system is performed, a cluster test must be added setting, in XML block, the node <internalParallel> to True.
6. If the change modifies or adds a requirement or a requirement based test case, the Change Control Board's Chair or designee also needs to approve the change. The requirements and the requirements test shall be in sync.
7. The merge request must reference an issue. If the issue is closed, the issue close checklist shall be done.
8. If an analytic test is changed/added is the the analytic documentation updated/added?
9. If any test used as a basis for documentation examples (currently found in raven/tests/framework/user_guide and raven/docs/workshop) have been changed, the associated documentation must be reviewed and assured the text matches the example.

ravenframework/Models/PostProcessors/BasicStatistics.py

…/idaholab/raven into aalfonsi/subgrid_basicStatistics

mandd · 2023-05-23T22:39:05Z

doc/user_manual/PostProcessors/SubdomainBasicStatistics.tex

+
+In this case, the RAVEN variables ``mean\_x01, mean\_x02, sen\_x01\_a, sen\_x02\_a, sen\_x01\_b, sen\_x02\_b''
+will be computed for each cell defined above (2 cells in this case).
+The results will be associated to the mid-point of each point. For the example above:


The results will be associated to the mid-point of each cell

mandd · 2023-05-23T22:39:58Z

doc/user_manual/PostProcessors/SubdomainBasicStatistics.tex

+In this case, the RAVEN variables ``mean\_x01, mean\_x02, sen\_x01\_a, sen\_x02\_a, sen\_x01\_b, sen\_x02\_b''
+will be computed for each cell defined above (2 cells in this case).
+The results will be associated to the mid-point of each point. For the example above:
+\begin{itemize}


should this be:
\item $(a=40.0,b=30.0)$
\item $(a=70.0,b=30.0)$
no?

The interval of the fist grid is
[10,100] with 2 steps:
|---------|---------|
10 55 100
midpoints:
------|--------|------
32.5 77.5

mandd · 2023-05-23T22:40:49Z

doc/user_manual/raven_user_manual.tex

+\setlist[itemize,7]{label=$\bullet$}
+\setlist[itemize,8]{label=$\bullet$}
+\renewlist{itemize}{itemize}{8}
+


what is the purpose of this added code?

I added a comment at the begin of the command:
the following allows for 8 levels list nesting

This allows to add an additional level of nesting for the "itemize" command

mandd · 2023-05-23T22:42:02Z

doc/user_manual/sampler.tex

+    For example, if the grid \xmlAttr{type} is \xmlString{value}, in the body
+    of \xmlNode{grid}, the user will specify the values representing the nodes of the grid.
+\end{itemize}
+}


why is this description needed in sampler.tex?

yeah probably not the right location. Moved to postprocessors.tex

mandd · 2023-05-23T22:42:31Z

doc/user_manual/sampler.tex

@@ -234,6 +262,21 @@ \section{Samplers}
  information is requested.}
 }

+\newcommand{\gridDescriptionNoCFD}
+{


what does CFD stand for?

Typo. CFD -> CDF (Cumulative Dist Function)

mandd · 2023-05-24T14:15:18Z

I have left my initial set of comments. One of my main questions is why a new PP class was added rather than extending the directly the basic statistics?

doc/user_manual/PostProcessors/SubdomainBasicStatistics.tex

ravenframework/Models/PostProcessors/SubdomainBasicStatistics.py

…/idaholab/raven into aalfonsi/subgrid_basicStatistics

aalfonsi · 2023-05-24T16:18:30Z

I have left my initial set of comments. One of my main questions is why a new PP class was added rather than extending the directly the basic statistics?

@mandd addressed your comments. The reason is mostly because it would have required an additional refactoring of the basic statistics and it is already quite complex. The advantage of having this PP is that the computation is encapsulated in the BasicStatistics and, in the future, the calculation for each "cell" can be parallelized if needed.

moosebuild · 2023-05-24T16:18:46Z

Job Mingw Test on 6297969 : invalidated by @aalfonsi

mandd · 2023-05-25T20:03:57Z

doc/user_manual/PostProcessors/SubdomainBasicStatistics.tex

+In this case, the RAVEN variables ``mean\_x01, mean\_x02, sen\_x01\_a, sen\_x02\_a, sen\_x01\_b, sen\_x02\_b''
+will be computed for each cell defined above (2 cells in this case).
+The results will be associated to the mid-point of each point. For the example above:
+\begin{itemize}


wangcj05 · 2023-05-25T20:57:40Z

Hi @aalfonsi: Is it possible that the subdomain information (the grid lower and upper bound) also get stored in the DataObjects, for example in the metadata? I would recommend to preserve this information in the output DataObject.

aalfonsi · 2023-05-25T22:04:22Z

Hi @aalfonsi: Is it possible that the subdomain information (the grid lower and upper bound) also get stored in the DataObjects, for example in the metadata? I would recommend to preserve this information in the output DataObject.

@wangcj05 can you open an issue and I can address it in a followup MR?

aalfonsi added 4 commits May 10, 2023 16:39

subdomain basic stats

e953892

updated grid entity

e225c0c

added tests

a44ea7f

added documentations

d3e4c21

aalfonsi requested a review from mandd May 22, 2023 19:10

removed unused imports

169c687

aalfonsi commented May 22, 2023

View reviewed changes

ravenframework/Models/PostProcessors/BasicStatistics.py Outdated Show resolved Hide resolved

Apply suggestions from code review

a262164

aalfonsi added the Ready To Review label May 22, 2023

aalfonsi added 2 commits May 22, 2023 14:32

readded self.outputMultipleRealizations

12024fd

Merge branch 'aalfonsi/subgrid_basicStatistics' of https://github.com…

da52bc7

…/idaholab/raven into aalfonsi/subgrid_basicStatistics

aalfonsi requested a review from wangcj05 May 23, 2023 18:58

wangcj05 added the RAVENv3.0 label May 23, 2023

mandd requested changes May 24, 2023

View reviewed changes

aalfonsi commented May 24, 2023

View reviewed changes

doc/user_manual/PostProcessors/SubdomainBasicStatistics.tex Outdated Show resolved Hide resolved

Update doc/user_manual/PostProcessors/SubdomainBasicStatistics.tex

138c038

aalfonsi commented May 24, 2023

View reviewed changes

ravenframework/Models/PostProcessors/SubdomainBasicStatistics.py Outdated Show resolved Hide resolved

aalfonsi added 3 commits May 24, 2023 08:41

Update ravenframework/Models/PostProcessors/SubdomainBasicStatistics.py

36bf939

moved latex outside sampler

b0a2f97

Merge branch 'aalfonsi/subgrid_basicStatistics' of https://github.com…

6297969

…/idaholab/raven into aalfonsi/subgrid_basicStatistics

aalfonsi requested a review from mandd May 24, 2023 17:29

mandd approved these changes May 25, 2023

View reviewed changes

mandd merged commit 2295f8f into devel May 25, 2023

mandd deleted the aalfonsi/subgrid_basicStatistics branch May 25, 2023 22:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Subdomain basic statistics . #2119

Subdomain basic statistics . #2119

aalfonsi commented May 22, 2023 •

edited by mandd

Loading

mandd May 23, 2023

aalfonsi May 24, 2023

mandd May 23, 2023

aalfonsi May 24, 2023

mandd May 25, 2023

mandd May 23, 2023

aalfonsi May 24, 2023

mandd May 23, 2023

aalfonsi May 24, 2023

mandd May 23, 2023

aalfonsi May 24, 2023

mandd commented May 24, 2023

aalfonsi commented May 24, 2023

moosebuild commented May 24, 2023

mandd May 25, 2023

wangcj05 commented May 25, 2023

aalfonsi commented May 25, 2023

Subdomain basic statistics . #2119

Subdomain basic statistics . #2119

Conversation

aalfonsi commented May 22, 2023 • edited by mandd Loading

Pull Request Description

What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)

What are the significant changes in functionality due to this change request?

For Change Control Board: Change Request Review

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

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

mandd commented May 24, 2023

aalfonsi commented May 24, 2023

moosebuild commented May 24, 2023

Choose a reason for hiding this comment

wangcj05 commented May 25, 2023

aalfonsi commented May 25, 2023

aalfonsi commented May 22, 2023 •

edited by mandd

Loading