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

Cannot load abstractions after clustring #32

Open
MaxFstl opened this issue Jan 23, 2025 · 14 comments
Open

Cannot load abstractions after clustring #32

MaxFstl opened this issue Jan 23, 2025 · 14 comments

Comments

@MaxFstl
Copy link

MaxFstl commented Jan 23, 2025

Hey I started training 5 days ago, but unfortunately my clustring crashed after loading the metric for the flop. When I tried to restart the clustering and loading in the saved abstractions, I get the error:
thread '<unnamed>' panicked at src/clustering/metric.rs:24:14: missing abstraction pair

I don't know why this is the case, but during clustering i got the message abstraction cluster RMS error a couple of times.
I also used --features shortdeck and changed the bucket sizes, if that has any impact on the bug.

Here is the log for reference:
22:55:17 [INFO] initialize kmeans river
22:55:17 [INFO] clustering kmeans river
22:55:17 [INFO] calculating metric river
22:55:17 [INFO] saving metric river
22:55:17 [INFO] calculating lookup river
23:01:01 [INFO] saving lookup river
23:01:23 [INFO] calculating transitions river
23:01:23 [INFO] saving transition river
23:01:23 [INFO] loading lookup river
23:02:21 [INFO] loading metric river
23:02:22 [INFO] initialize kmeans turn
23:02:42 [INFO] clustering kmeans turn
23:03:04 [DEBUG] abstraction cluster RMS error 0.02535558
23:03:29 [DEBUG] abstraction cluster RMS error 0.021866854
23:03:55 [DEBUG] abstraction cluster RMS error 0.02136437
23:04:21 [DEBUG] abstraction cluster RMS error 0.021152442
23:04:47 [DEBUG] abstraction cluster RMS error 0.021053715
23:05:13 [DEBUG] abstraction cluster RMS error 0.020967714
23:05:39 [DEBUG] abstraction cluster RMS error 0.020908125
23:06:05 [DEBUG] abstraction cluster RMS error 0.020883476
23:06:31 [DEBUG] abstraction cluster RMS error 0.020864865
23:06:57 [DEBUG] abstraction cluster RMS error 0.020850323
23:06:57 [INFO] calculating metric turn
23:06:57 [INFO] saving metric turn
23:06:57 [INFO] calculating lookup turn
23:07:50 [INFO] saving lookup turn
23:07:54 [INFO] calculating transitions turn
23:07:54 [INFO] saving transition turn
23:07:55 [INFO] loading lookup turn
23:08:03 [INFO] loading metric turn
23:08:03 [INFO] initialize kmeans flop
02:07:24 [INFO] clustering kmeans flop
05:00:58 [DEBUG] abstraction cluster RMS error 0.050942555
17:48:52 [DEBUG] abstraction cluster RMS error 0.04658942
06:37:40 [DEBUG] abstraction cluster RMS error 0.045377757
19:28:41 [DEBUG] abstraction cluster RMS error 0.044904392
08:24:43 [DEBUG] abstraction cluster RMS error 0.044720482
21:24:06 [DEBUG] abstraction cluster RMS error 0.044641204
10:24:32 [DEBUG] abstraction cluster RMS error 0.044571135
23:30:46 [DEBUG] abstraction cluster RMS error 0.0445256
12:37:01 [DEBUG] abstraction cluster RMS error 0.044478342
01:44:48 [DEBUG] abstraction cluster RMS error 0.044429146
01:44:48 [INFO] calculating metric flop
01:58:09 [INFO] saving metric turn
01:58:09 [INFO] calculating lookup flop
15:05:08 [INFO] saving lookup flop
15:05:09 [INFO] calculating transitions flop
15:05:09 [INFO] saving transition flop
15:05:09 [INFO] loading lookup flop
15:05:09 [INFO] loading metric flop

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

yeah, this means that you ran one run with shortdeck on and the other with shortdeck off. basically, it will look up what the abstraction cluster is for a hand like 2s3c~4d5h6h and not find it because it was trained on hands 6+

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

RMS error is a measure of loss

@MaxFstl
Copy link
Author

MaxFstl commented Jan 23, 2025

yeah, this means that you ran one run with shortdeck on and the other with shortdeck off

I ran both with cargo run --features shortdeck. I am unfamiliar with cargo and Rust but I think that turns on shortdeck mode.

RMS error is a measure of loss

Oh that makes sense :D
Thought it is an error message

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

hm interesting -- there is another possibility, which is that i changed the src code for the Abstraction data structure in between your two runs. did you update source code at any point between the two?

a stack trace could maybe be helpful

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

also what new bucket sizes are you using? maybe could be helpful to see the set of consts in lib.rs

@MaxFstl
Copy link
Author

MaxFstl commented Jan 23, 2025

did you update source code at any point between the two?

I don't think I did. But the first run also crashed after loading the metrics. I don't have the detailed error message though.

a stack trace could maybe be helpful

How can I do that?

also what new bucket sizes are you using? maybe could be helpful to see the set of consts in lib.rs

I used a KMEANS_CLUSTER_COUNT of 32 for each flop, turn and equity and used 10 Training iterations for each.
The rest was the default configuration.

I am currently running it again with a lower configuration. Maybe I can reconstruct the error.

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

ahhh very useful to know. yes indeed you've surfaced a known edge case that i've largely ignored since i didn't really expect anyone to play around with the parameters that much, but i ought to patch. give me a minute to get to my desk but i know what happened and how to fix

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

okay so basically this is downstream of a (very suboptimal) way of serializing the Metric data structure to disk.

  • when we create a Metric, it's basically just a BTreeMap of unordered Abstraction pairs.
  • each metric is also associated with a Street, but this isn't actually stored in the data structure.
  • the way that we calculate &Metric -> Street is very hacky. we basically say "does this BTreeMap have the (N_CLUSTERS_ON_STREET choose 2) number of entries as we'd expect?" so this (very undesirably) requires that each street has a unique number of clusters. read the comment in the link, it'll clarify a bit
  • the correct way to do this is to obviously not require this constraint, but rather just to store the single byte of Street data in the header of the PGCOPY serialization. just a bit at the top of each file that says "this is the (Preflop, Flop, ...) Metric"
  • so what likely happened is that either 1) you loaded a Metric from disk and the application thinks its one street, when it's really another or 2) you wrote a Metric to disk that was associated with one street, but saved as another.

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

it's probably an LLM one-shot of a fix that i just havent gotten around to since i've had the pgcopy files locally for quite awhile and havent had to run the clustering algorithm from scratch in a few weeks.

  • struct Metric needs to have a field for Street
  • as a result, it needs to change it's construction via From<BTreeMap> to From<(BTreeMap, Street)>
  • Metric::street() then becomes a simple one-liner
  • Metric::save() will need to write this byte in the postgres header
  • Metric::load() will need to read this byte from the postgres header

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

does this make sense

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

i should also mention, the much easier fix is simply to use a unique value for each of the N_CLUSTERS_* for each street!

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

there's a way in which you can avoid having to re-cluster, if you make the proper fix and then modify the header bytes of existing pgcopy so that future load() calls work as desired. but otherwise you may have to re-run clustering,

@MaxFstl
Copy link
Author

MaxFstl commented Jan 23, 2025

Thanks for the fast response. I think I will try to use smaller bucket sizes and make them unique.

@krukah
Copy link
Owner

krukah commented Jan 23, 2025

i recommend also keeping the N_EQUITY_CLUSTERS parameter higher than 32, something like 101 is good since it will bucket every river hand into a percentile bucket. the calculations for the river layer are very very efficient, so it certainly won't be the bottleneck if you wanted to have higher resolution at that layer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants