Addition of SCALE CSAS sequence in scale interface #2155

alfoa · 2023-07-21T16:08:59Z

Pull Request Description

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

Closes #2154

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

Addition of the CSAS sequence in SCALE interface.
Updated manual and added test

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.

wangcj05 · 2023-07-21T17:00:12Z

@alfoa FYI
##########################################################################
ERROR: The following files contain trailing whitespace after applying your patch:
ravenframework/CodeInterfaceClasses/SCALE/ScaleInterface.py

Run the "delete_trailing_whitespace.sh" script in your $MOOSE_DIR/scripts directory.
##########################################################################

alfoa · 2023-07-21T17:04:44Z

@alfoa FYI ########################################################################## ERROR: The following files contain trailing whitespace after applying your patch: ravenframework/CodeInterfaceClasses/SCALE/ScaleInterface.py

Run the "delete_trailing_whitespace.sh" script in your $MOOSE_DIR/scripts directory. ##########################################################################

@wangcj05 fixed

wangcj05 · 2023-07-21T17:56:15Z

ravenframework/CodeInterfaceClasses/SCALE/csasData.py

+      @ In, None
+      @ Out, None
+    """
+    self._data['time'] = [0.]


why time is zero here? If there is only one value for time, why do you need time in your output?

(I thought that if a user uses HistorySets, RAVEN will complain because the pivot parameter (time) is not found. I can remove it if you want)

I think that depends on the CSAS code. Will a time-dependent outputs generated by CSAS code? If so, I think the code interface need to be revised in order to correctly process it. I would suggest to remove the time for now. @alfoa

@wangcj05 removed

…om/idaholab/raven into alfoa/scale_interface_augmentation

alfoa · 2023-07-24T16:55:55Z

@wangcj05 it seems that the plugins tests time out in HERON (>6 hrs testing time)

moosebuild · 2023-07-27T14:22:32Z

All jobs on fed0eee : invalidated by @wangcj05

restart all tests for recent fix in test scripts.

wangcj05

I have some comments for you to review. Thanks for your contribution. @alfoa

wangcj05 · 2023-07-27T15:54:31Z

ravenframework/Distributions.py

+      if self.isFloat:
+        idx = bisect.bisect(list(self.values), x)
+        pdfValue = self.mapping[list(self.values)[idx]]
+      else:
+        self.raiseAnError(IOError,'Categorical distribution cannot calculate pdf for ' + str(x))


@alfoa These lines of code are not clear to me. I think the bisect will work only for sorted list, for example the bisect for unsorted list:

In addition, thanks for the updates on the extension of outcome, they can be either float or string, it seems our code only accept float values, however, I think there are several other places that you also need to make changes, such as "ppf"method, currently it only return float values, "cdf" method still require it to be float, and our manual also need to be updated if we extend it to string value. Could you make those changes?

moosebuild · 2023-07-27T19:00:35Z

Job Test mac on fed0eee : invalidated by @wangcj05

Timeout tests/framework/ROM/TimeSeries/ARMA/ZeroFilter

moosebuild · 2023-07-27T22:17:45Z

Job Test mac on fed0eee : invalidated by @wangcj05

set python environment issue

…om/idaholab/raven into alfoa/scale_interface_augmentation

moosebuild · 2023-07-28T18:59:16Z

Job Test qsubs sawtooth on ec78e32 : invalidated by @alfoa

moosebuild · 2023-07-28T19:30:00Z

Job Test mac on ec78e32 : invalidated by @wangcj05

time out for (524F1/870) Timeout(300.05sec)tests/framework/ROM/TimeSeries/ARMA/ZeroFilter

moosebuild · 2023-07-28T21:33:25Z

Job Test qsubs sawtooth on ec78e32 : invalidated by @wangcj05

fetch issue

wangcj05 · 2023-07-28T21:34:19Z

There is a consistently timeout for test: (524F1/870) Timeout(300.06sec)tests/framework/ROM/TimeSeries/ARMA/ZeroFilter

moosebuild · 2023-07-28T21:42:35Z

Job Test mac on ec78e32 : invalidated by @alfoa

moosebuild · 2023-07-28T21:44:18Z

Job Test qsubs sawtooth on ec78e32 : invalidated by @alfoa

moosebuild · 2023-07-31T14:28:45Z

Job Test qsubs sawtooth on ec78e32 : invalidated by @wangcj05

fetch issue

alfoa · 2023-07-31T15:26:10Z

There is a consistently timeout for test: (524F1/870) Timeout(300.06sec)tests/framework/ROM/TimeSeries/ARMA/ZeroFilter

:(

moosebuild · 2023-07-31T16:16:38Z

Job Test qsubs sawtooth on ec78e32 : invalidated by @joshua-cogliati-inl

Failed with Device or resource busy

moosebuild · 2023-07-31T16:42:08Z

Job Test mac on ec78e32 : invalidated by @alfoa

alfoa · 2023-08-01T18:54:13Z

There is a consistently timeout for test: (524F1/870) Timeout(300.06sec)tests/framework/ROM/TimeSeries/ARMA/ZeroFilter

@wangcj05 this MR doesn't touch anything that should affect the ARMA. ARMA uses Normal and Multivariate distributions that are not impacted on my Categorical distribution changes.

moosebuild · 2023-08-03T16:14:51Z

Job Test mac on ec78e32 : invalidated by @alfoa

Try again. ARMA is time-outing

removed un-used imports

moosebuild · 2023-08-03T16:37:40Z

Job Test qsubs sawtooth on 946178f : invalidated by @alfoa

alfoa · 2023-08-03T17:28:04Z

There is a consistently timeout for test: (524F1/870) Timeout(300.06sec)tests/framework/ROM/TimeSeries/ARMA/ZeroFilter

@wangcj05 this MR doesn't touch anything that should affect the ARMA. ARMA uses Normal and Multivariate distributions that are not impacted on my Categorical distribution changes.

@wangcj05 can you advice on the issue with ARMI zeroFiltering test?

moosebuild · 2023-08-03T17:28:10Z

Job Test qsubs sawtooth on 946178f : invalidated by @alfoa

moosebuild · 2023-08-06T21:28:47Z

Job Test qsubs sawtooth on 946178f : invalidated by @wangcj05

fetch failed

wangcj05 · 2023-08-06T21:34:47Z

ravenframework/Distributions.py

+        try:
+          float(outcome)
+          self.isFloat = True
+        except:
+          self.isFloat = False
+        if outcome in self.values:
          self.raiseAnError(IOError,'Categorical distribution has identical outcomes')
        else:
-          self.values.add(float(outcome))
+          self.values.add(float(outcome) if self.isFloat else outcome)
      else:


@alfoa: This is a general comment, and do not need to address in this PR:

The assumption here is that the users can either provide float or string outcomes. If in the inputs, there are mixed floats and strings, the current implementation can not catch it, and an unexpected crash will happen. I would suggest add a check here to make sure the inputs are either all floats or all string values.

wangcj05

PR looks good, and I have left one general comments for you to consider.

wangcj05 · 2023-08-06T21:36:15Z

@alfoa: In addition, there is an issue with Mac test which is not related to this PR.

moosebuild · 2023-08-08T16:47:36Z

Job Test mac on 946178f : invalidated by @wangcj05

wangcj05 · 2023-08-08T16:50:17Z

@alfoa FYI, we have fixed the issue in Mac test machine. For this week, Sawtooth is under maintenance. Please let us know if you want this PR to be merged before that.

alfoa · 2023-08-08T16:56:22Z

@alfoa FYI, we have fixed the issue in Mac test machine. For this week, Sawtooth is under maintenance. Please let us know if you want this PR to be merged before that.

@wangcj05 there is no rush. We can wait next week (we will need this feature ~7/10 days from now). I will also address your additional comment (you are right there can be a very bad crash hehe..I ll address it :) thanks a lot )

joshua-cogliati-inl · 2023-08-08T16:57:15Z

If we do merge it while sawtooth is down, make sure to manually run: ./developer_tools/pylint_check and ./doc/make_docs.sh to check that those work with the pull request.

wangcj05 · 2023-08-08T19:03:42Z

@joshua-cogliati-inl Once sawtooth is back and the tests are green, you can merge it.

moosebuild · 2023-08-09T03:38:34Z

Job Test qsubs sawtooth on 946178f : invalidated by @wangcj05

aalfonsi added 3 commits July 19, 2022 08:21

categorical with ls

5125630

csas

ff9ddfe

updated scale interface

16ce407

alfoa requested a review from wangcj05 July 21, 2023 16:09

aalfonsi added 2 commits July 21, 2023 11:01

no csv anymore

d50d68c

remove trailing spaces

28ab891

aalfonsi and others added 2 commits July 21, 2023 11:10

added other outputs

511982a

Update test_scale_csas_output.xml

a9483f7

wangcj05 requested changes Jul 21, 2023

View reviewed changes

aalfonsi added 2 commits July 23, 2023 17:36

remove time

a7034f6

Merge branch 'alfoa/scale_interface_augmentation' of https://github.c…

815cd87

…om/idaholab/raven into alfoa/scale_interface_augmentation

alfoa requested a review from wangcj05 July 23, 2023 23:37

Update scale.tex

fed0eee

wangcj05 requested changes Jul 27, 2023

View reviewed changes

aalfonsi and others added 5 commits July 28, 2023 10:40

addressed Congjian's comments

d748436

Update Distributions.py

c9d7964

fixed type in raven

3ca6806

Merge branch 'alfoa/scale_interface_augmentation' of https://github.c…

495b068

…om/idaholab/raven into alfoa/scale_interface_augmentation

ravenframework/Distributions.py

ec78e32

Update csasData.py

946178f

removed un-used imports

wangcj05 reviewed Aug 6, 2023

View reviewed changes

wangcj05 approved these changes Aug 6, 2023

View reviewed changes

wangcj05 added the RAVENv3.0 label Aug 6, 2023

wangcj05 requested a review from joshua-cogliati-inl August 8, 2023 17:00

joshua-cogliati-inl merged commit 87626d6 into devel Aug 9, 2023

joshua-cogliati-inl deleted the alfoa/scale_interface_augmentation branch August 9, 2023 15:10

Addition of SCALE CSAS sequence in scale interface #2155

Addition of SCALE CSAS sequence in scale interface #2155

Conversation

alfoa commented Jul 21, 2023 • edited by joshua-cogliati-inl 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

wangcj05 commented Jul 21, 2023

alfoa commented Jul 21, 2023

wangcj05 Jul 21, 2023

Choose a reason for hiding this comment

alfoa Jul 21, 2023

Choose a reason for hiding this comment

wangcj05 Jul 23, 2023

Choose a reason for hiding this comment

alfoa Jul 23, 2023

Choose a reason for hiding this comment

alfoa commented Jul 24, 2023 • edited Loading

moosebuild commented Jul 27, 2023

wangcj05 left a comment

Choose a reason for hiding this comment

wangcj05 Jul 27, 2023

Choose a reason for hiding this comment

alfoa Jul 28, 2023

Choose a reason for hiding this comment

moosebuild commented Jul 27, 2023

moosebuild commented Jul 27, 2023

moosebuild commented Jul 28, 2023

moosebuild commented Jul 28, 2023

moosebuild commented Jul 28, 2023

wangcj05 commented Jul 28, 2023

moosebuild commented Jul 28, 2023

moosebuild commented Jul 28, 2023

moosebuild commented Jul 31, 2023

alfoa commented Jul 31, 2023

moosebuild commented Jul 31, 2023

moosebuild commented Jul 31, 2023

alfoa commented Aug 1, 2023 • edited Loading

moosebuild commented Aug 3, 2023

moosebuild commented Aug 3, 2023

alfoa commented Aug 3, 2023

moosebuild commented Aug 3, 2023

moosebuild commented Aug 6, 2023

wangcj05 Aug 6, 2023

Choose a reason for hiding this comment

wangcj05 left a comment

Choose a reason for hiding this comment

wangcj05 commented Aug 6, 2023

moosebuild commented Aug 8, 2023

wangcj05 commented Aug 8, 2023

alfoa commented Aug 8, 2023 • edited Loading

joshua-cogliati-inl commented Aug 8, 2023

wangcj05 commented Aug 8, 2023

moosebuild commented Aug 9, 2023

alfoa commented Jul 21, 2023 •

edited by joshua-cogliati-inl

Loading

alfoa commented Jul 24, 2023 •

edited

Loading

alfoa commented Aug 1, 2023 •

edited

Loading

alfoa commented Aug 8, 2023 •

edited

Loading