Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wallSpeedLimit looks incorrect #115

Closed
arouinfar opened this issue Jun 10, 2019 · 4 comments
Closed

wallSpeedLimit looks incorrect #115

arouinfar opened this issue Jun 10, 2019 · 4 comments

Comments

@arouinfar
Copy link
Contributor

In reviewing the performance for #98, I noticed that the wallSpeedLimit for compression looks incorrect in dev.36. It appears quite a bit faster than dev.32 which had the appropriate speed limit. Not sure if the compression speed limit was correct when I reviewed #90 (comment) in master, but I believe it was.

@pixelzoom
Copy link
Contributor

I'll investigate. dev.32 and dev.36 should both have wallSpeedLimit: 800 (decision in #90 (comment)), and that's what it's currently set to in master:

    // Speed limit for the container's left movable wall, in pm/ps. Relevant when reducing the container size.
    // For internal use only.
    wallSpeedLimit: {
      type: 'number',
      isValidValue: value => ( value > 0 ),
      defaultValue: 800
    }

@pixelzoom
Copy link
Contributor

pixelzoom commented Jun 10, 2019

I reproduced the problem - dev.36 and master are indeed faster than dev.32.

dev.35 looks correct, so something changed between dev.35 (6/5/19 @ 7:18pm) and dev.36. (6/5/19 @ 9:35pm).

There's only one relevant changeset between dev.35 and dev.36: a7c7a54

@pixelzoom
Copy link
Contributor

Fixed in the above commit. There were some ofter changes, and it looks like perhaps the container was getting stepped twice, sorry about that. @arouinfar please verify in master or 1.0.0-dev.37.

@arouinfar
Copy link
Contributor Author

Looking good in dev.37, thanks @pixelzoom!

pixelzoom added a commit that referenced this issue Jun 10, 2019
pixelzoom added a commit that referenced this issue Jun 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants