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

fix: fix concurrency problems #10

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

fix: fix concurrency problems #10

wants to merge 20 commits into from

Conversation

bioinformatist
Copy link
Owner

@bioinformatist bioinformatist commented Sep 23, 2023

@lazyky 义父,please assist me in completing this PR. The tasks that need to be done are as follows:

  • Replace all instances of derive-builder with typed-builder.
  • Rewrite the unit test for the Inference trait to cover the scenario of two threads simultaneously operating on the same batch (you can refer to the Register trait for guidance).
  • Use tempfile::NamedTempFile for temporary files.
  • Replace the type of working_status, which is a property of TaskConfig, with &[&str], which is the same as the signature of Inference::inference.
  • Write a new simple test_task_running_in_parallel() (just leave the comments there, I'll add examples with Burn later).
  • The Metadata now uses Option<i64> instead of a regular i64 type for the model_update_time field. Please make sure to update this in the Inference trait accordingly.
  • The data_path field should be removed everywhere.
  • The number of fields and tags can be determined by Vec<T>, rendering the relative fields obsolete in both the Register and Inference trait.

@bioinformatist bioinformatist added the enhancement New feature or request label Sep 25, 2023
* test: add tests for Task and Inference

- add test_task_running_in_parallel()
- rewrite the test of Inference
- update deadpool
- change vec<&str> into &[&str]
@bioinformatist
Copy link
Owner Author

@lazyky There are still some issues with the unit test for Task. Using an async function is unnecessary in this case. Additionally, since we are using anyhow::Result, why not use ? everywhere?

bioinformatist and others added 7 commits October 7, 2023 14:52
* refactor: delete unnecessary code

- Use ? for `anyhow::Result`
- delete unnecessary `async`

* refactor: refactor the use of asynchronous
* feat: Optimize the timing of task run()

- Change model_update_time tag in Stable training_data to train_start_time
- Add update_train_start_time() to update by timestamp of latest available model
- Delete model_update_time in Metadata
@bioinformatist
Copy link
Owner Author

@lazyky It is recommended to merge structure TaskConfig into TDengine as other kind of database may need different configurations.

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

Successfully merging this pull request may close these issues.

2 participants