fix optimizer writeStep final (if optimization ends for reaching the limit number of iterations) #2387

alfoa · 2024-10-21T18:42:29Z

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?

Added a print of the out streams at the end of the finalizeSampler call in the MultiRun step to make sure that the "final" solution is printed if an out stream is requested in a MultiRun step

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.

…is stopped not for a convergence but for reaching the limit)

ravenframework/Steps/MultiRun.py

…b/raven into alfoa/fixOptimizerWriteStepFinal

alfoa · 2024-10-23T17:29:08Z

@wangcj05 @mandd ready to be reviewed

moosebuild · 2024-10-23T19:04:34Z

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

failed in fetch fatal: fetch-pack: invalid index-pack output

moosebuild · 2024-10-23T19:51:13Z

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

failed in fetch fatal: fetch-pack: invalid index-pack output

tests/framework/Optimizers/GeneticAlgorithms/discrete/unconstrained/testGAMaxwoRepConvAHDp.xml

…ained/testGAMaxwoRepConvAHDp.xml

moosebuild · 2024-10-23T21:58:38Z

Job Mingw Test on e8b15fd : invalidated by @alfoa

moosebuild · 2024-10-23T21:58:57Z

Job Test qsubs sawtooth on e8b15fd : invalidated by @alfoa

moosebuild · 2024-10-24T13:54:24Z

Job Mingw Test on e8b15fd : invalidated by @alfoa

alfoa · 2024-10-24T15:11:36Z

@joshua-cogliati-inl ray test fails in Windows (but this merge request should not influence that test). Is it a random failure?

joshua-cogliati-inl · 2024-10-25T15:16:29Z

@joshua-cogliati-inl ray test fails in Windows (but this merge request should not influence that test). Is it a random failure?

Yes, the ray test is not the most reliable test, so that is probably random.

moosebuild · 2024-10-25T16:02:59Z

Job Mingw Test on e8b15fd : invalidated by @alfoa

…b/raven into alfoa/fixOptimizerWriteStepFinal

alfoa · 2024-10-25T21:44:19Z

dependencies.xml

@@ -68,7 +68,7 @@ Note all install methods after "main" take
    <dask source="pip" pip_extra="[complete]"/>
    <ray source="pip" pip_extra="[default]">2.6</ray>
    <!-- redis is needed by ray, but on windows, this seems to need to be explicitly stated -->
-    <redis source="pip" os='windows'/>
+    <redis source="pip" os='windows'>5.1</redis>


This is required because redis released a new version on Oct 24 2024 (5.2) and this new version is not compatible with ray 2.6 (causing the Windows test to fail).

See https://pypi.org/project/redis/#history

well changing the dependency does not help.... @joshua-cogliati-inl no idea

mandd · 2024-10-30T04:32:40Z

dependencies.xml

@@ -68,7 +68,7 @@ Note all install methods after "main" take
    <dask source="pip" pip_extra="[complete]"/>
    <ray source="pip" pip_extra="[default]">2.6</ray>
    <!-- redis is needed by ray, but on windows, this seems to need to be explicitly stated -->
-    <redis source="pip" os='windows'/>
+    <redis source="pip" os='windows'>5.1.0</redis>


is there a reason behind the choice of this particular version?

yeah...trying out the latest version of redis that seemed to work . Now I tried another one

mandd · 2024-10-30T04:33:52Z

ravenframework/OutStreams/PrintInterfaces/FilePrint.py

@@ -148,6 +148,8 @@ def run(self):
      if self.options['type'] == 'csv':
        filename = dictOptions['filenameroot']
        rlzIndex = self.indexPrinted.get(filename,0)
+        if rlzIndex and rlzIndex >= len(self.sourceData[index]):


I am not sure if these two lines are needed

this is needed when the printing of the dataset is not finished (e.g. point set) and this is triggered in the multi run step (right before exiting the step). We check here the index and the length in case the printing is finished before reaching the ned of the step

mandd · 2024-10-30T04:38:26Z

...orithms/gold/continuous/unconstrained/metaModelWithCodeAndFunctionsAndGenetic/opt_export.csv

@@ -1,22 +1,22 @@
 trajID,sigma-A,sigma-B,decay_A,decay_B,sum,age,batchId,fitness,iteration,accepted,AHDp,conv_AHDp


while the previous csv contains new rows the the bottom of the file as expected, this file contains some differences thorughout the rows, any possible explanation here?

the rows are swapped.

moosebuild · 2024-11-01T14:02:53Z

Job Mingw Test on 50828f1 : invalidated by @alfoa

wangcj05 · 2024-11-04T05:12:20Z

ravenframework/Steps/MultiRun.py

+    for myLambda, outIndex in self._outputCollectionLambda:
+      if isinstance(outputs[outIndex], OutStreamEntity):
+        myLambda([None,outputs[outIndex]])
+        self.raiseAMessage(f'Finalized output "{inDictionary["Output"][outIndex].name}"')


@alfoa The proposed changes can resolve the issue. However, it can be confusing since these lines are almost similar to collection part in the same function. Could you provide more details why previous collection can not collect the final solution? Is it possible to make some modifications inside the optimizer to enable it? I have two concerns for the proposed approach:

Two collections in the same function, which make it very confusing. Either add more explanations or find a way to avoid it.

It is also very confusing in the FilePrint.py, since the new added lines to check the rlzIndex seem very unnecessary. I see the changes make the final collection possible, but it is really hard to understand why these lines are needed unless the developers fully understand the collections in the steps.

The lines in the FilePrint are not unnecessary. They are required when the collection in triggered on data objects that are not "collected/created" by the Optimizers.

Basically, the "SolutionExport" in the Optimizers is "updated" with the final solution after the collection is triggered (at the begin of the processing of the "last job"). So the Outstream is not invoked before exiting the Multirun loop.

This modification was the "minimal viable" solution to trigger an out stream call at the end of the multi run.

Maybe another approach could be to split the calls to the output collection:

Right after a Job is finished for data objects (DataObjects/Databases)

After the call to the finalizeActualSampling for OutStreams

…using

dependencies.xml

alfoa · 2024-12-12T18:36:53Z

This issue is being solved in #2326. I can close this PR.

alfoa added 2 commits October 21, 2024 12:04

Closes #2386

d2f4ec3

regolded GA that did not print final solution (when the optimization …

254f49a

…is stopped not for a convergence but for reaching the limit)

alfoa requested a review from wangcj05 October 21, 2024 18:42

alfoa commented Oct 21, 2024

View reviewed changes

ravenframework/Steps/MultiRun.py Show resolved Hide resolved

Update ravenframework/Steps/MultiRun.py

90536b0

alfoa added the Ready To Review label Oct 21, 2024

alfoa requested a review from mandd October 21, 2024 18:44

alfoa added Do Not Merge and removed Ready To Review labels Oct 21, 2024

alfoa added 3 commits October 23, 2024 08:51

if the printing is done, don't print

2b68e5c

Merge branch 'alfoa/fixOptimizerWriteStepFinal' of github.com:idahola…

ff47a1a

…b/raven into alfoa/fixOptimizerWriteStepFinal

xml

39c5a39

alfoa added Ready To Review and removed Do Not Merge labels Oct 23, 2024

alfoa requested a review from joshua-cogliati-inl October 23, 2024 20:21

alfoa commented Oct 23, 2024

View reviewed changes

tests/framework/Optimizers/GeneticAlgorithms/discrete/unconstrained/testGAMaxwoRepConvAHDp.xml Outdated Show resolved Hide resolved

Update tests/framework/Optimizers/GeneticAlgorithms/discrete/unconstr…

e8b15fd

…ained/testGAMaxwoRepConvAHDp.xml

alfoa added 2 commits October 25, 2024 15:41

test pinned library

5fe5932

Merge branch 'alfoa/fixOptimizerWriteStepFinal' of github.com:idahola…

c1bdd2f

…b/raven into alfoa/fixOptimizerWriteStepFinal

alfoa commented Oct 25, 2024

View reviewed changes

trying with 5.1.0

d9c9dcc

mandd reviewed Oct 30, 2024

View reviewed changes

alfoa added 2 commits October 31, 2024 16:48

trying version of ray and redis that worked on windows

57d648c

trying version of ray and redis that worked on windows

50828f1

alfoa requested a review from mandd October 31, 2024 22:53

wangcj05 reviewed Nov 4, 2024

View reviewed changes

alfoa added Do Not Merge and removed Ready To Review labels Nov 4, 2024

changed approach to fix the issue. This is much cleaner and less conf…

e63c1e2

…using

alfoa added Ready To Review and removed Do Not Merge labels Nov 6, 2024

alfoa requested a review from wangcj05 November 6, 2024 21:58

alfoa commented Nov 6, 2024

View reviewed changes

dependencies.xml Outdated Show resolved Hide resolved

alfoa commented Nov 6, 2024

View reviewed changes

dependencies.xml Outdated Show resolved Hide resolved

Apply suggestions from code review

3164cbc

alfoa closed this Dec 12, 2024

alfoa deleted the alfoa/fixOptimizerWriteStepFinal branch December 12, 2024 18:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix optimizer writeStep final (if optimization ends for reaching the limit number of iterations) #2387

fix optimizer writeStep final (if optimization ends for reaching the limit number of iterations) #2387

alfoa commented Oct 21, 2024 •

edited

Loading

alfoa commented Oct 23, 2024

moosebuild commented Oct 23, 2024

moosebuild commented Oct 23, 2024

moosebuild commented Oct 23, 2024

moosebuild commented Oct 23, 2024

moosebuild commented Oct 24, 2024

alfoa commented Oct 24, 2024

joshua-cogliati-inl commented Oct 25, 2024

moosebuild commented Oct 25, 2024

alfoa Oct 25, 2024

alfoa Oct 28, 2024

mandd Oct 30, 2024

alfoa Oct 31, 2024

mandd Oct 30, 2024

alfoa Oct 31, 2024

mandd Oct 30, 2024

alfoa Oct 31, 2024

moosebuild commented Nov 1, 2024

wangcj05 Nov 4, 2024

alfoa Nov 4, 2024 •

edited

Loading

alfoa commented Dec 12, 2024

		@@ -1,22 +1,22 @@
		trajID,sigma-A,sigma-B,decay_A,decay_B,sum,age,batchId,fitness,iteration,accepted,AHDp,conv_AHDp

fix optimizer writeStep final (if optimization ends for reaching the limit number of iterations) #2387

fix optimizer writeStep final (if optimization ends for reaching the limit number of iterations) #2387

Conversation

alfoa commented Oct 21, 2024 • edited 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

alfoa commented Oct 23, 2024

moosebuild commented Oct 23, 2024

moosebuild commented Oct 23, 2024

moosebuild commented Oct 23, 2024

moosebuild commented Oct 23, 2024

moosebuild commented Oct 24, 2024

alfoa commented Oct 24, 2024

joshua-cogliati-inl commented Oct 25, 2024

moosebuild commented Oct 25, 2024

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

moosebuild commented Nov 1, 2024

Choose a reason for hiding this comment

alfoa Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

alfoa commented Dec 12, 2024

alfoa commented Oct 21, 2024 •

edited

Loading

alfoa Nov 4, 2024 •

edited

Loading