Closed adegroote closed 8 years ago
Your test will be correct if you use time.sleep(1.0) everywhere, not simu.sleep(1.0) (which waits for one second on simulated time).
What I'm doing in the current test is using always the same simulated time (so the displacement is the same), but show that it does not correspond to the same real-time
I see. IMHO, I think it would be valuable to have a test similar to what I proposed (but with time.sleep(1)
, indeed), as it seems easier to understand to me -> possibly closer to what people will try to achieve in their applications.
But we can certainly have both.
It would be nice to have plenty of comments so that people can easily use the unit-test as a doc/example.
Pushed a new version with automatic adjustment of fps (and few typos fixed). I will improve the test later, through I guess there are equivalent
I rework the test to handle your remarks. Please review.
There is two yet-open issues regarding this:
Well, my first thought is that, if you slow down the time, then you slow down the time! Hence, you also slow down the sensors -> I would expect to receive less updates from the sensors. So, no adjustement of the FPS. I guess some may have different use cases, I would keep it as the default.
For the builder, I would prefer to have a dedicated set_time_scale
(with optionally explicit keyword arguments slowdown_by
and accelerate_by
for extra clarity). The meaning of set_time_strategy
is not obvious without reading the doc, and I don't think we should make it even more complex.
Well, my first thought is that, if you slow down the time, then you slow down the time! Hence, you also slow down the sensors -> I would expect to receive less updates from the sensors. So, no adjustement of the FPS. I guess some may have different use cases, I would keep it as the default.
The current code makes automatic adjustment in d5d6af086da413c084f1a565deb9e866e3b80fbe. Otherwise, you will receive a lot more data, as fps must be understand by Blender as frame by real second. Slowing side is the easy side, accelerating one is more complex (you can reach easily computing resource limit). Moreover, going faster than real-time is often to test "high-level" "long-term" algorithm, so in some situation, you do not need to have sensor update at 60Hz (simulated-time) (so 300Hz real-time if you have x5 factor).
On 15/01/16 10:38, Arnaud Degroote wrote:
The current code makes automatic adjustment in d5d6af0 https://github.com/morse-simulator/morse/commit/d5d6af086da413c084f1a565deb9e866e3b80fbe. Otherwise, you will receive a lot more data, as fps must be understand by Blender as frame by real second. Slowing side is the easy side, accelerating one is more complex (you can reach easily computing resource limit). Moreover, going faster than real-time is often to test "high-level" "long-term" algorithm, so in some situation, you do not need to have sensor update at 60Hz (simulated-time) (so 300Hz real-time if you have x5 factor).
Well, that's a good point indeed.
I take in account your remarks, and add some builder interface.
For automatic fps adjustment, I suggest to keep the code as itself, and document that the fps set at the Morse level is always the number of frame by simulated second.
Once the minor doc wording points are addressed, the PR looks good to me.
So it is ok to merge it before I continue to work on #683 ?
Merged
It is the first attempt to handle #338. You need blender-current (and python 3.5) to test it.
I still have doubt on the interface.
set_time_scale for moment only changes time_scale, and not fps. fps in blender is really frame by real second. So it means that if you set the time_scale by two, you will get sensor information at 30Hz (simulated time). It is possible to automatically adjust fps to have frame by simulated second constant (but we may need to raise max_logic_step / max_physics_step), and we need to handle #683 too. I suppose it can raise problem too if we have a really low time_scale too (but who really need that).
Your through ?