-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Remove outliers when calculating BSQ rate #4706
Remove outliers when calculating BSQ rate #4706
Conversation
Add percentage for upper and lower threshold for outlier detection.
When I click the DAO menu, I get a jfoenix / reflection related NPE:
It's happened after three startups -- two of them on a fresh data directory. I do think it is related to the current I am using OpenJDK 12 on Ubuntu 12. |
Hm... I am not aware of any change which would be related to jfoenix... Can you try if you get the exception also on master? |
This is a jFoenix issue with OpenJDK 12 quite sure. Should be gone when #4242 is merged and we use the newer jFoenix version. |
I tried your PR's branch again, this time downgrading from JDK 12 to JDK 11, and there was no NPE. I got the same results in the master branch. Clicking the DAO menu item in:
|
@ghubstan Since it works on JDK 11, do you give this an ack? |
ACK |
I'm not seeing a difference in BSQ rates displayed on This is from v1.4.2: This is from master @ 00725cf: Note that the Market cap and Total available BSQ values do differ. Not sure if that's related to this change. |
I also noticed that when running latest master against a fresh data directory (i.e. when emulating a brand-new user's first run of Bisq), that the Facts & Figures Dashboard what appear to be incomplete data: This is v1.4.2 running against a clean data dir: And this is latest master against a clean data dir: Notice how under latest master both BSQ/BTC and USD/BSQ the 30- and 90-day values are identical, and how few data points there are on the chart. Is this a known issue? |
Thanks for finding the issue. Its caused by #4628. Working on a bugfix.... |
Fixed with #4745 |
Thanks, @chimp1984. As I just mentioned at #4745 (comment), I've tested this locally, and I can see that the fix works. However, there is still a problem that goes back to my original comment on this PR above at #4706 (comment). The following screenshots are of two different Bisq instances running against latest master at 63cae1c, i.e. with PR #4745 merged. Notice the discrepancy between the average trade price values: This is master running against an fresh data directory: And this is master running against my actual data directory, i.e. my personal Bisq client as I usually run it: The former appears to have the outliers removed as intended, while the latter does not. The latter displays roughly the same values as it did against v1.4.2. Note that DAO synchronization is complete on both of the above clients. |
@cbeams Can you check at settings/dao options what the outlier % is there? Maybe it got set to 0 at the update and if its 0 it does not do anything. 5% is default. I tested with fresh apps and it worked that 5% was initial value. |
Yes, this was the problem. It was set to 0% in my existing Bisq app after the upgrade. I manually set it to 5% and got the expected behavior. |
Hm... need to test again for the update scenario, it should take the 5% by default as well but might be a bug |
Yes was a bug, fixed in #4747 |
Remove outliers when calculating BSQ rate.
Add percentage for upper and lower threshold for outlier detection.
It filters outliers in the BSQ and in the USD market.
Settings:
As one can see below even a small % threshold removes the few outliers we had recently: 5 % is default value.
With 0%:
With 1%:
With 5%:
With 10%:
With 20%:
With 49% (max):