Addition of datatypes for R5 interface #2212

alfoa · 2023-11-24T05:16:18Z

Pull Request Description

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

Closes #2211

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

Addition of the XML node to specify datatypes.

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.

mandd

One additional comment: I think we do not check that the format of the sampled variables match the format specified in the input file

mandd · 2023-11-27T18:03:07Z

ravenframework/CodeInterfaceClasses/RELAP5/RELAPparser.py

@@ -431,10 +440,18 @@ def modifyOrAdd(self,modifyDict,save=True):
        temp.append('*'+' deckNum: '+str(deckNum)+'\n')
      for j in sorted(modiDictionaryList):
        for var in modiDictionaryList[j]:
+          ff = '{:7e}'
+          cast = float
+          if j in intDT and int(intDT[j]) == int(var['position']):


since we have two datatypes, should we specify only one of the datatypes in the input files?

yeah..since float is the default, I remover the input for them (redundant)

mandd · 2023-11-27T18:04:11Z

ravenframework/CodeInterfaceClasses/RELAP5/RELAPparser.py

+            ff = '{:d}'
+            cast = int
+          elif j in floatDT and int(floatDT[j]) == int(var['position']) :
+            ff = '{:7e}'


should we mention this float format in the user manual?

RELAP does not use a fix-typed input file so probably it is not needed.

mandd · 2023-11-27T18:06:15Z

ravenframework/CodeInterfaceClasses/RELAP5/RELAPparser.py

@@ -462,12 +479,18 @@ def modifyOrAdd(self,modifyDict,save=True):
      # modify the cards
      for card in cardLines.keys():
        for var in modiDictionaryList[card]:
+          dtype = "float"
+          if card in intDT and int(intDT[card]) == int(var['position']):


why do we need the second portion of this if condition (int(intDT[card]) == int(var['position']))?

yes that is needed because multiple "cards" can be inputted at different position. For example, we can sample 2 cards: 201:1 and 201:2 and we want only the second one to be integer. Ie.:
<integers>201:2</integers>
If we do not check for the position, both cards will be treated as integers.

mandd · 2023-11-27T18:10:08Z

ravenframework/CodeInterfaceClasses/RELAP5/Relap5Interface.py

@@ -317,13 +329,15 @@ def createNewInput(self,currentInputFiles,oriInputFiles,samplerType,**Kwargs):
             where RAVEN stores the variables that got sampled (e.g. Kwargs['SampledVars'] => {'var1':10,'var2':40})
      @ Out, newInputFiles, list, list of newer input files, list of the new input files (modified and not)
    """
+    if "_indexMap" in Kwargs['SampledVars']:


why these two lines have been added?

this is because this "variable" is added in the custom sampler and in RELAP5 any variable in SampledVars are treated as variables to be changed

alfoa · 2023-12-04T03:41:12Z

@mandd I addressed all the comments.

alfoa · 2023-12-27T13:13:09Z

@mandd it should be green now

moosebuild · 2024-01-02T22:40:47Z

Job Test qsubs sawtooth on 6c938ae : invalidated by @alfoa

alfoa · 2024-01-02T22:41:08Z

@mandd this is ready for re-review

moosebuild · 2024-01-04T15:53:16Z

Job Test qsubs sawtooth on 6c938ae : invalidated by @wangcj05

python environment issue

moosebuild · 2024-01-23T22:11:23Z

Job Test qsubs sawtooth on 6c938ae : invalidated by @mandd

moosebuild · 2024-01-23T23:28:43Z

Job Test qsubs sawtooth on 6c938ae : invalidated by @mandd

moosebuild · 2024-01-24T23:16:49Z

Job Test qsubs sawtooth on 6c938ae : invalidated by @mandd

moosebuild · 2024-02-29T15:46:39Z

Job Test qsubs sawtooth on 6c938ae : invalidated by @wangcj05

Failed tests/framework/pca_rom/oneDim

wangcj05

I have a couple of comments for you to consider @alfoa

wangcj05 · 2024-03-01T17:32:17Z

ravenframework/CodeInterfaceClasses/RELAP5/RELAPparser.py

+    elif dtype == 'float':
+      ff, cast = '{:7e}', float


may not need this elif conditions here.

completely right. Fixed

wangcj05 · 2024-03-01T17:37:08Z

ravenframework/CodeInterfaceClasses/RELAP5/Relap5Interface.py

+      elif child.tag == 'datatypes':
+        # DATA TYPES
+        integers = child.find("integers")
+        if integers is not None:
+          c = "," if "," in integers.text else None
+          self.datatypes['integers'] = [e.strip() for e in (integers.text.split() if c is None else integers.text.split(c))]


only integers are processed here. I would suggest to add a check here, I would suggest to check all the subnodes under datatypes to prevent any typos or other types provided by users.

completely right. Fixed

wangcj05 · 2024-03-01T17:41:03Z

ravenframework/CodeInterfaceClasses/RELAP5/RELAPparser.py

    """
      Constructor
      @ In, inputFile, string, input file name
+      @ In, datatypes, {}, dict {floats:[],integers:[]}


The default is floats, and the codes only process integers and it seems to me only integers are stored in datatypes. I would suggest update the line with integers only.

wangcj05

changes are good.

wangcj05 · 2024-03-07T20:38:51Z

checklist is good.

alfoa added 2 commits November 23, 2023 21:54

new datatypes

7ff9290

documentation

f78a0bb

alfoa assigned mandd Nov 24, 2023

alfoa added 2 commits November 23, 2023 22:30

added test

e9989e1

removed trailing

53ce1e7

mandd reviewed Nov 27, 2023

View reviewed changes

alfoa added 2 commits November 29, 2023 23:33

fix if multiple cards

21187b8

addressed comments

8dc0ab7

alfoa requested a review from mandd December 4, 2023 03:41

alfoa added 2 commits December 4, 2023 11:13

Removed trailing

9a041fb

fixed gold

6c938ae

wangcj05 requested changes Mar 1, 2024

View reviewed changes

alfoa added 3 commits March 6, 2024 17:11

Merge branch 'devel' into alfoa/relap5_datatypes

8f59096

removed floats since only integers are supported

6b2d6ae

removed redondant if statement

d96e763

alfoa requested a review from wangcj05 March 7, 2024 00:26

wangcj05 approved these changes Mar 7, 2024

View reviewed changes

wangcj05 merged commit b0e0faa into devel Mar 7, 2024
12 checks passed

wangcj05 deleted the alfoa/relap5_datatypes branch March 7, 2024 20:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addition of datatypes for R5 interface #2212

Addition of datatypes for R5 interface #2212

alfoa commented Nov 24, 2023 •

edited by wangcj05

Loading

mandd left a comment

mandd Nov 27, 2023

alfoa Dec 4, 2023

mandd Nov 27, 2023

alfoa Dec 4, 2023

mandd Nov 27, 2023

alfoa Dec 4, 2023

mandd Nov 27, 2023

alfoa Dec 4, 2023

alfoa commented Dec 4, 2023

alfoa commented Dec 27, 2023

moosebuild commented Jan 2, 2024

alfoa commented Jan 2, 2024

moosebuild commented Jan 4, 2024

moosebuild commented Jan 23, 2024

moosebuild commented Jan 23, 2024

moosebuild commented Jan 24, 2024

moosebuild commented Feb 29, 2024

wangcj05 left a comment

wangcj05 Mar 1, 2024

alfoa Mar 7, 2024

wangcj05 Mar 1, 2024

alfoa Mar 7, 2024

wangcj05 Mar 1, 2024

alfoa Mar 7, 2024

wangcj05 left a comment

wangcj05 commented Mar 7, 2024

Addition of datatypes for R5 interface #2212

Addition of datatypes for R5 interface #2212

Conversation

alfoa commented Nov 24, 2023 • edited by wangcj05 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

mandd 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfoa commented Dec 4, 2023

alfoa commented Dec 27, 2023

moosebuild commented Jan 2, 2024

alfoa commented Jan 2, 2024

moosebuild commented Jan 4, 2024

moosebuild commented Jan 23, 2024

moosebuild commented Jan 23, 2024

moosebuild commented Jan 24, 2024

moosebuild commented Feb 29, 2024

wangcj05 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

Choose a reason for hiding this comment

wangcj05 left a comment

Choose a reason for hiding this comment

wangcj05 commented Mar 7, 2024

alfoa commented Nov 24, 2023 •

edited by wangcj05

Loading