Feature/major refactor #12
Conversation
There was a problem hiding this comment.
nit: I feel like naming the files _client is redundant given they are in the clients folder and these would be imported as client[s].sklearn.
There was a problem hiding this comment.
I am not sure I understand the purpose of the model. Should a user generate the state_dict by hand?
nada_ai/utils/__init__.py
Outdated
There was a problem hiding this comment.
nit: I don't feel like neither the types nor the exceptions belong to "utils". I would rather include them as separate files or submodules. I think it makes more sense to import something like from nada_ai.typing import NillionType or from nada_ai.exceptions import MismatchedShapeException.
nada_ai/utils/typing.py
Outdated
There was a problem hiding this comment.
Also, thinking out loud. Would it make sense to group the typing between nada-algebra and nada-ai?
There was a problem hiding this comment.
Sure - I could see the value in having a nada_typing or nada_common at some point
There was a problem hiding this comment.
No major issues found. For the most part, what you propose are very logical changes to the project structure and improvements to code organization. I included a few "nits" but feel free to consider those if you feel they are reasonable. Some of them are probably not.
No new/different functionality. Also no changes to the user-facing interface.
Just a major refactor of the repo structure - this should avoid massive files & circular import issues in the future (e.g.
layers.pycould have easily grown to several thousand lines)