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

Wrong replay memory inclusion in fileio.py #189

Closed
yannikkellerde opened this issue Dec 11, 2022 · 1 comment
Closed

Wrong replay memory inclusion in fileio.py #189

yannikkellerde opened this issue Dec 11, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@yannikkellerde
Copy link

In _include_data_from_replay_memory in fileio.py the goal is to include random files from the most recent replay memory given by fraction_for_selection. For this, we get the filenames with os.listdir and as the code comment suggests, after inverting, the most recent files should be on top. However, os.listdir returns files in arbitrary order (see https://docs.python.org/3/library/os.html#os.listdir). I think the confusion comes from listdir being ordered by date on windows, but on my linux machine it definitely is not.

file_names = os.listdir(self.train_dir_archive)
# invert ordering (most recent files are on top)
file_names = file_names[::-1]

I'd suggest explicitly sorting via os.path.getmtime:
file_names.sort(key=lambda x:os.path.getmtime(os.path.join(self.train_dir_archive,x)))

@QueensGambit
Copy link
Owner

Sorry, for the long delay you are perfectly right that os.listdir returns files/folders in arbitrary order.
I now updated the code to extract the time stamp from the folder name and sort it by the time stamp.
This should be more robust compared to os.path.getmtime as it remains the same even when the folder has been moved.
Thanks again.

@QueensGambit QueensGambit added the bug Something isn't working label Jul 31, 2023
QueensGambit added a commit that referenced this issue Aug 8, 2023
* Add VIRTUAL_VISIT as new virtual simulation style

* Sort folder names in train archive by time stamp (close #189)

* Add virtualVisitIncrement

also only increase virtualLoss Counter by 1

* Add UCI-Option "Virtual_Visit_Increment"

* Add VIRTUAL_OFFSET UCI-Option

* Add missing ;

* Simplify VIRTUAL_VISIT

* Update virtual-Offset implementation

+ use also virtual-visit of 1
+ don't use additional vector operation

* Add missing implementation for VIRTUAL_OFFSET in
get_transposition_q_value()

* Change Centi_Virtual_Loss to Milli_Virtual_loss

* Change Milli_Virtual_Loss to Micro_Virtual_Loss

* Update max value for Micro_Virtual_Loss

* Add VIRTUAL_MIX
Rename VIRTUAL_LOSS into VIRTUAL_WEIGHT
Replace virtualVisitIncrement by virtualMixThreshold

* Use realVisitSum as condition for Virtual_Mix

* Simplify code and use realVisits of child node for VIRTUAL_MIX threshold

* Fix compile bugs

* Use d->childNumber visits again

* Deactive virtualWeight for now

* Deactive virtualWeight

* Use Q_INIT for comparision

* revert last change

* Add virtualOffsetStrenght(0.001)
Switch between VIRTUAL_OFFSET and VIRTUAL_VISIT when using VIRTUAL_MIX

* revert

* Update VIRTUAL_OFFSET

* Switch between VIRTUAL_LOSS and VIRTUAL_VISIT

* Add virtualLossIncrement

* remove virtualLossIncrement again due to underperformance

* Update UCI-default values

* Fix compile error

* Disable 960 Support for now due to problems

* Fix init of second argument in first_and_second_max()

* Remove init of additional nodes in NodeData

* Remove Virtual_Weight for now
QueensGambit added a commit that referenced this issue Aug 8, 2023
* Add VIRTUAL_VISIT as new virtual simulation style

* Sort folder names in train archive by time stamp (close #189)

* Add virtualVisitIncrement

also only increase virtualLoss Counter by 1

* Add UCI-Option "Virtual_Visit_Increment"

* Add VIRTUAL_OFFSET UCI-Option

* Add missing ;

* Simplify VIRTUAL_VISIT

* Update virtual-Offset implementation

+ use also virtual-visit of 1
+ don't use additional vector operation

* Add missing implementation for VIRTUAL_OFFSET in
get_transposition_q_value()

* Change Centi_Virtual_Loss to Milli_Virtual_loss

* Change Milli_Virtual_Loss to Micro_Virtual_Loss

* Update max value for Micro_Virtual_Loss

* Add VIRTUAL_MIX
Rename VIRTUAL_LOSS into VIRTUAL_WEIGHT
Replace virtualVisitIncrement by virtualMixThreshold

* Use realVisitSum as condition for Virtual_Mix

* Simplify code and use realVisits of child node for VIRTUAL_MIX threshold

* Fix compile bugs

* Use d->childNumber visits again

* Deactive virtualWeight for now

* Deactive virtualWeight

* Use Q_INIT for comparision

* revert last change

* Add virtualOffsetStrenght(0.001)
Switch between VIRTUAL_OFFSET and VIRTUAL_VISIT when using VIRTUAL_MIX

* revert

* Update VIRTUAL_OFFSET

* Switch between VIRTUAL_LOSS and VIRTUAL_VISIT

* Add virtualLossIncrement

* remove virtualLossIncrement again due to underperformance

* Update UCI-default values

* Fix compile error

* Disable 960 Support for now due to problems

* Fix init of second argument in first_and_second_max()

* Remove init of additional nodes in NodeData

* Remove Virtual_Weight for now

* Fix first_and_second_max()

Update secondMax = std::numeric_limits<T>::min();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants