CRZbulabula commented on code in PR #15756:
URL: https://github.com/apache/iotdb/pull/15756#discussion_r2155585518
##########
iotdb-core/ainode/ainode/core/manager/cluster_manager.py:
##########
@@ -15,32 +15,163 @@
# specific language governing permissions and limitations
# under the License.
#
+import threading
Review Comment:
Revert all changes of this file, do not make this PR more complex
##########
iotdb-core/ainode/ainode/core/config.py:
##########
@@ -155,10 +155,33 @@ def set_ain_target_config_node_list(self,
ain_target_config_node_list: str) -> N
ain_target_config_node_list
)
+ def get_support_iotdb_models(self) -> bool:
+ """Check whether IoTDB model format is supported"""
+ return getattr(self, "_support_iotdb_models", True)
+
+ def set_support_iotdb_models(self, support: bool) -> None:
+ """Set whether to support IoTDB model format"""
+ self._support_iotdb_models = support
+
+ def get_model_loading_timeout(self) -> int:
+ """Get model loading timeout (in seconds)"""
+ return getattr(self, "_model_loading_timeout", 300)
+
+ def set_model_loading_timeout(self, timeout: int) -> None:
+ """Set model loading timeout (in seconds)"""
+ self._model_loading_timeout = timeout
+
+ def get_auto_model_format_detection(self) -> bool:
+ """Check whether automatic model format detection is enabled"""
+ return getattr(self, "_auto_model_format_detection", True)
+
+ def set_auto_model_format_detection(self, auto_detect: bool) -> None:
+ """Set whether to enable automatic model format detection"""
+ self._auto_model_format_detection = auto_detect
+
Review Comment:
Remove these useless config parameters
##########
iotdb-core/ainode/ainode/core/manager/inference_manager.py:
##########
@@ -50,32 +50,71 @@ def infer(self, full_data, **kwargs):
pass
-# [IoTDB] full data deserialized from iotdb is composed of [timestampList,
valueList, length],
-# we only get valueList currently.
+# [IoTDB] Full data deserialized from IoTDB is composed of [timestampList,
valueList, length],
Review Comment:
I think all changes in this file is irrelevant with the goal of this PR, so
restore this file
##########
iotdb-core/ainode/ainode/core/model/safetensor_loader.py:
##########
@@ -0,0 +1,266 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from pathlib import Path
Review Comment:
After reading ur updates in model_storage.py, it seems that this file is
useless now?
##########
iotdb-core/ainode/ainode/core/constant.py:
##########
@@ -53,15 +53,44 @@
TRIAL_ID_PREFIX = "__trial_"
DEFAULT_TRIAL_ID = TRIAL_ID_PREFIX + "0"
-DEFAULT_MODEL_FILE_NAME = "model.safetensors"
-DEFAULT_CONFIG_FILE_NAME = "config.json"
+
+DEFAULT_MODEL_FILE_NAME = "model.pt"
+DEFAULT_CONFIG_FILE_NAME = "config.yaml"
Review Comment:
Our project should support the .safetensors by default
##########
iotdb-core/ainode/ainode/core/manager/model_manager.py:
##########
@@ -112,6 +252,68 @@ def get_ckpt_path(self, model_id: str) -> str:
"""
return self.model_storage.get_ckpt_path(model_id)
+ def _validate_model_name(self, model_name: str) -> bool:
+ """
+ Validate the model name format
+
+ Args:
+ model_name: Name of the model
+
+ Returns:
+ Whether the name is valid
+ """
+ if not model_name:
+ return False
+
+ import re
+
+ pattern = r"^[a-zA-Z0-9_-]+$"
+
+ if re.match(pattern, model_name):
+ logger.debug(f"Model name validated: {model_name}")
+ return True
+ else:
+ logger.error(f"Illegal model name: {model_name}")
+ return False
Review Comment:
We don't need to verify the legality of the model name, cuz the SQL parser
in IoTDB has already done this. Maybe we should add this feature in the
furture, but not today. Thus, this interface should be removed.
##########
iotdb-core/ainode/ainode/core/manager/model_manager.py:
##########
@@ -42,63 +51,194 @@
class ModelManager:
def __init__(self):
self.model_storage = ModelStorage()
+ self._model_status_cache = {} # Cache for model statuses
+ self._status_lock = ReadWriteLock()
Review Comment:
Currently, the ConfigNode is in charge of the model status, while the AINode
only store the model weights and utlize models. It's true that we might
optimize this situation, yet this modification should not be this PR.
Therefore, plz check and remove all status handle logic
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
