Fix plots

[pnkov]

Contributor License Agreement

This repository (optuna-dashboard) and Goptuna share common code.
This pull request may therefore be ported to Goptuna.
Make sure that you understand the consequences concerning licenses and check the box below if you accept the term before creating this pull request.

• I agree this patch may be ported to Goptuna by other Goptuna contributors.

Reference Issues/PRs

Fixes #232

What does this implement/fix? Explain your changes.

This PR fixes plots by sorting trials and intermediate values. For me the issue appeared because select row order is not guaranteed to be the same as insert order (if order by is not provided) in case of postgres storage. And plotly expects values to be in sequential order to draw lines connected one by another.

[c-bata]

@pnkov Thank you for your pull request! As you confirmed, the internal implementation of RDBStorage seems not ensure the order of intermediate values. We need to sort them by step attributes.

To make the API client easy to read, I think it's better to sort intermediate values at server side. Could you sort values at serialize_intermediate_values() function, not client-side?

optuna-dashboard/optuna_dashboard/_serializer.py

Lines 59 to 63 in 40858d2

	def serialize_intermediate_values(values: Dict[int, float]) -> List[IntermediateValue]:
	return [
	{"step": step, "value": "inf" if math.isinf(value) else value}
	for step, value in values.items()
	]

[c-bata]

Thank you for the update! The order of trials are actually ensured in all storages. So we don't need to sort trials here. Could you revert this change?

[pnkov]

My edf looks like this without this change:

And like this with sorting:

Do you need additional info about my setup?

[c-bata]

@pnkov Hm. We might had better file a bug report to Optuna. I have two questions.

1. Which storage did you use? (e.g. optuna.storages.RDBStorage with PostgreSQL)
2. Could you check the output of following code?

>>> import optuna
>>> storage = optuna.storages.get_storage('your-storage-url')
>>> [t.number for t in storage.get_all_trials(study_id=...)]
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99]
>>> [t._trial_id for t in storage.get_all_trials(study_id=...)]
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100]

[pnkov]

I'm using docker image so it's optuna.storages.RDBStorage. It turns out sorting is done only in CachedStorage. So what should I do next?

>>> import optuna
>>> storage = optuna.storages.get_storage('postgresql+psycopg2://postgres:secret@postgres/postgres')
>>> [t.number for t in storage.get_all_trials(study_id=12)]
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99]
>>> [t._trial_id for t in storage.get_all_trials(study_id=12)]
[353, 354, 355, 356, 357, 358, 359, 360, 362, 363, 364, 365, 367, 368, 369, 370, 372, 373, 374, 376, 377, 378, 379, 380, 381, 382, 383, 384, 385, 386, 387, 388, 389, 390, 391, 392, 393, 395, 396, 397, 398, 399, 400, 401, 402, 403, 404, 405, 406, 407, 408, 409, 410, 411, 413, 414, 415, 416, 417, 418, 419, 420, 421, 422, 423, 424, 425, 426, 427, 428, 429, 430, 431, 433, 434, 435, 436, 437, 439, 440, 441, 442, 443, 444, 446, 447, 448, 449, 450, 451, 452, 453, 454, 455, 456, 457, 458, 459, 460, 461]
>>> storage.__class__
<class 'optuna.storages._cached_storage._CachedStorage'>

>>> storage = optuna.storages.RDBStorage('postgresql+psycopg2://postgres:secret@postgres/postgres')
>>> [t.number for t in storage.get_all_trials(study_id=12)]
[33, 34, 28, 38, 5, 36, 1, 11, 10, 9, 8, 7, 14, 24, 42, 32, 17, 40, 43, 41, 49, 25, 54, 58, 48, 83, 85, 87, 94, 62, 97, 98, 93, 90, 35, 44, 2, 4, 19, 12, 15, 22, 23, 52, 29, 30, 53, 60, 55, 64, 47, 66, 67, 63, 68, 70, 77, 79, 81, 82, 92, 99, 88, 72, 31, 39, 0, 65, 6, 95, 13, 37, 16, 78, 20, 18, 3, 27, 96, 26, 21, 51, 80, 73, 56, 59, 61, 45, 57, 74, 46, 50, 71, 69, 75, 84, 89, 76, 86, 91]
>>> [t._trial_id for t in storage.get_all_trials(study_id=12)]
[390, 391, 385, 396, 358, 393, 354, 365, 364, 363, 362, 360, 369, 381, 400, 389, 373, 398, 401, 399, 407, 382, 413, 417, 406, 444, 447, 449, 456, 421, 459, 460, 455, 452, 392, 402, 355, 357, 376, 367, 370, 379, 380, 410, 386, 387, 411, 419, 414, 423, 405, 425, 426, 422, 427, 429, 437, 440, 442, 443, 454, 461, 450, 431, 388, 397, 353, 424, 359, 457, 368, 395, 372, 439, 377, 374, 356, 384, 458, 383, 378, 409, 441, 433, 415, 418, 420, 403, 416, 434, 404, 408, 430, 428, 435, 446, 451, 436, 448, 453]
>>> storage.__class__
<class 'optuna.storages._rdb.storage.RDBStorage'>

[c-bata]

@pnkov Thank you for your investigation. We should file a bug report to Optuna. Could you open an issue at Optuna repository if you have time?
https://github.com/optuna/optuna/issues/new?assignees=&labels=bug&template=bug-report.yml

I'll merge this PR after fixed flake8 errors. I guess CI will be passed after executing $ pip install black && black optuna_dashboard.

[c-bata]

Hi @pnkov. I merged this PR at #244 with some improvements. Thank you for your contribution!