Skip to content

Conversation

snadampal
Copy link

torch allows setting the interop thread count only once and before any inter-op parallel work is started because it creates threadpool statically.
To follow this constraint, check the interop thread count and set only if it's not yet set.

@snadampal snadampal changed the title fix crash during torch model reload with inter op thread paramter set fix crash during torch model reload with inter op thread parameter set Sep 29, 2025
Copy link

@gerilya gerilya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During model reload the call to set inter_op value will be skipped altogether, however, nothing will be logged.
From the user perspective, it might create a confusion as to what inter_op value is used internally by Pytorch backend.
I would suggest taking LOG_MESSAGE out of the conditon, so inter_op_thread_count value is always logged whether setting it skipped or not.

torch allows setting the interop thread count only once
and before any inter-op parallel work is started because
it creates threadpool statically.
To follow this constraint, check the interop thread count
and set only if it's not yet set.
@snadampal snadampal force-pushed the fix_crash_during_model_reload branch from 46e7f68 to b5a7410 Compare September 30, 2025 14:06
@snadampal
Copy link
Author

@gerilya thanks for the feedback. I have made the LOG_MESSAGE unconditional so the inter_op_thread_count value set at torch is always logged. I hope this removes the confusion of what value is being used.

} else {
if (inter_op_thread_count > 0) {
if ((inter_op_thread_count > 0) &&
(inter_op_thread_count != at::get_num_interop_threads()) &&
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can still allow a repeated call if two model config files set the number of interop threads to different values. (See #159 for a safer solution.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. thanks for fixing the multiple config file scenario as well!

@snadampal
Copy link
Author

closing this in favor of #159

@snadampal snadampal closed this Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants