[GUI] Improve advanced timer #468

chrijopa · 2017-10-12T12:01:03Z

This pull request allows the usage of the advanced timer

In the standard way
runSofa -c 1
Only for the init functions:
runSofa -c 0
For the init function and for every second step:
runSofa -c -2

Moreover, the usage of the advanced timer in batch mode works again:
runSofa -c -2 -g batch -n 10

Fixes #357

This PR:

builds with SUCCESS for all platforms on the CI.
does not generate new warnings.
does not generate new unit test failures.
does not generate new scene test failures.
does not break API compatibility.
is more than 1 week old (or has fast-merge label).

Reviewers will merge only if all these checks are true.

sofabot · 2017-10-12T12:01:06Z

Thank you for your pull request!
Someone will soon check it and start the builds.

untereiner · 2017-10-17T15:58:55Z

applications/projects/runSofa/Main.cpp

@@ -185,7 +185,7 @@ int main(int argc, char** argv)
    bool        noAutoloadPlugins = false;
    int         nbIterations = BatchGUI::DEFAULT_NUMBER_OF_ITERATIONS;
    unsigned int nbMSSASamples = 1;
-    unsigned    computationTimeSampling=0; ///< Frequency of display of the computation time statistics, in number of animation steps. 0 means never.
+    int    computationTimeSampling=0; ///< Frequency of display of the computation time statistics, in number of animation steps. 0 means never.


where is the computationTimeSampling variable negative ?
I can't find it

damienmarchal · 2017-10-18T09:28:19Z

Hello,

Thank for the PR. The added feature is nice and should be merged but the way it hack the command line by allowing "-2" is very confusing.

Why not doing:
The standard way
runSofa -c 1

Only for the init functions:
runSofa -b
(as -i is already taken maybe -b to begin is a good choice)

For the init function and for every second step:
runSofa -c 2 -b

It would clearly separate the concerns instead of encoding different behavior in the same variable.

chrijopa · 2017-10-19T20:50:44Z

Thank you Damien for the important remark considering "-2", I was not aware of this problem. I will look into further details tomorrow.

…init

jnbrunet · 2017-11-03T14:52:34Z

applications/projects/runSofa/Main.cpp

+        sofa::helper::AdvancedTimer::setEnabled("Init", true);
+        sofa::helper::AdvancedTimer::setInterval("Init", 1);
+        sofa::helper::AdvancedTimer::setOutputType("Init", computationTimeOutputType);
+        sofa::helper::AdvancedTimer::begin("Init");


There is a duplicated begin("Init") here

There was a bug in the advanced timer (variable initialization at -1 instead of 0), that I just removed. Should be fine but it is good if that is double checked.

hugtalbot · 2017-11-06T17:06:53Z

@chrijopa could you take some time to progress on this ? this is open for a long time

guparan · 2017-11-15T09:32:31Z

[ci-build]

guparan · 2017-11-22T09:22:18Z

applications/sofa/gui/BatchGUI.cpp

+        std::cout << "Computing "<<nbIter<<" iterations." << std::endl;
+        sofa::helper::AdvancedTimer::begin("Animate");
+        sofa::helper::AdvancedTimer::end("Animate");
+        sofa::helper::AdvancedTimer::begin("Animate");


@chrijopa: Why begin then end immediately?

guparan · 2017-11-22T09:22:53Z

applications/sofa/gui/BatchGUI.cpp

        sofa::simulation::getSimulation()->animate(groot.get());
+        std::cout << sofa::helper::AdvancedTimer::end("Animate", groot.get()) << std::endl;


Please use msg_info instead of std::cout

guparan · 2017-11-22T09:23:30Z

applications/projects/runSofa/Main.cpp

    sofa::simulation::getSimulation()->init(groot.get());
+    if( computationTimeAtBegin )
+    {
+        std::cout << sofa::helper::AdvancedTimer::end("Init", groot.get()) << std::endl;


Please use msg_info instead of std::cout

…sg_info

…putationTimeSampling # Conflicts: # applications/projects/runSofa/Main.cpp # applications/sofa/gui/BatchGUI.cpp

guparan · 2017-12-12T10:46:25Z

[ci-build]

guparan · 2017-12-12T10:51:57Z

[ci-build]

hugtalbot · 2017-12-20T09:20:50Z

@ErwanDouaille could you check (very shortly) the API of argument parser in this PR please? we would like to integrate it in v17.12 release ;)

hugtalbot · 2018-01-03T09:21:17Z

[ci-build][with-scene-tests]

ErwanDouaille · 2018-01-03T11:03:32Z

@hugtalbot I didn't test it but the argument parser stuff looks good to me. I would suggest to not define b/o characters because, AFAIK, advance timer is not often used so maybe there is better use of b/o character.

chrisjopau added 2 commits October 12, 2017 13:53

[Advanced Timer] call at initialization works

dbf8437

[Advanced Timer] in batch mode

edb9597

hugtalbot added enhancement About a possible enhancement pr: status to review To notify reviewers to review this pull-request labels Oct 12, 2017

hugtalbot requested a review from untereiner October 12, 2017 15:17

untereiner reviewed Oct 17, 2017

View reviewed changes

guparan added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Oct 18, 2017

chrisjopau added 2 commits October 20, 2017 11:36

[AdvancedTimer] output type from command line

f95636b

[AdvancedTimer] changed usage: -b for computation time statistics at …

cc8bacb

…init

hugtalbot changed the title ~~Improve advanced timer~~ [GUI] Improve advanced timer Nov 2, 2017

jnbrunet reviewed Nov 3, 2017

View reviewed changes

guparan added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Nov 15, 2017

guparan reviewed Nov 22, 2017

View reviewed changes

guparan suggested changes Nov 22, 2017

View reviewed changes

guparan added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Nov 22, 2017

chrisjopau and others added 3 commits December 11, 2017 19:54

[AdvancedTimer] removed bug that forced calling init twice; use now m…

c7e6171

…sg_info

Merge remote-tracking branch 'sofa-framework/master' into improve_com…

61b7bc5

…putationTimeSampling # Conflicts: # applications/projects/runSofa/Main.cpp # applications/sofa/gui/BatchGUI.cpp

[SofaGui] Using msg_info API

b72467f

[SofaGui] FIX misplaced quote

18e8f99

guparan approved these changes Dec 19, 2017

View reviewed changes

guparan added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Dec 19, 2017

ErwanDouaille self-requested a review January 3, 2018 09:19

sofa-framework deleted a comment from sofabot Jan 3, 2018

hugtalbot added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Jan 10, 2018

guparan merged commit 573fc41 into sofa-framework:master Jan 14, 2018

untereiner deleted the improve_computationTimeSampling branch January 14, 2018 18:50

guparan added this to the v17.12 milestone Jan 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GUI] Improve advanced timer #468

[GUI] Improve advanced timer #468

chrijopa commented Oct 12, 2017 •

edited by guparan

Loading

sofabot commented Oct 12, 2017

untereiner Oct 17, 2017

damienmarchal commented Oct 18, 2017

chrijopa commented Oct 19, 2017

jnbrunet Nov 3, 2017

chrijopa Dec 11, 2017

hugtalbot commented Nov 6, 2017

guparan commented Nov 15, 2017

guparan Nov 22, 2017

guparan Nov 22, 2017

guparan Nov 22, 2017

guparan commented Dec 12, 2017

guparan commented Dec 12, 2017

hugtalbot commented Dec 20, 2017

hugtalbot commented Jan 3, 2018

ErwanDouaille commented Jan 3, 2018

		sofa::simulation::getSimulation()->animate(groot.get());
		std::cout << sofa::helper::AdvancedTimer::end("Animate", groot.get()) << std::endl;

[GUI] Improve advanced timer #468

[GUI] Improve advanced timer #468

Conversation

chrijopa commented Oct 12, 2017 • edited by guparan Loading

sofabot commented Oct 12, 2017

untereiner Oct 17, 2017

Choose a reason for hiding this comment

damienmarchal commented Oct 18, 2017

chrijopa commented Oct 19, 2017

jnbrunet Nov 3, 2017

Choose a reason for hiding this comment

chrijopa Dec 11, 2017

Choose a reason for hiding this comment

hugtalbot commented Nov 6, 2017

guparan commented Nov 15, 2017

guparan Nov 22, 2017

Choose a reason for hiding this comment

guparan Nov 22, 2017

Choose a reason for hiding this comment

guparan Nov 22, 2017

Choose a reason for hiding this comment

guparan commented Dec 12, 2017

guparan commented Dec 12, 2017

hugtalbot commented Dec 20, 2017

hugtalbot commented Jan 3, 2018

ErwanDouaille commented Jan 3, 2018

chrijopa commented Oct 12, 2017 •

edited by guparan

Loading