-
Notifications
You must be signed in to change notification settings - Fork 8
Expand docs with architecture overview #22
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
Expand docs with architecture overview #22
Conversation
…horough-documentation-with-theme-customization
EiffL
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a small comment on the config.py
Also the reason why I didn't include the image and spectum in the manager test is that it was taking too much memory at some point and was crashing the test. At least I think that was the problem... If it works now that's good, but if the tests crash at some point that would probably be why.
aion/codecs/config.py
Outdated
| "class": ImageCodec, | ||
| "repo_id": "polymathic-ai/aion-image-codec", | ||
| }, | ||
| LegacySurveyImage: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @LTMeyer but it was on purpose that I didn't include the LegacySurveyImage here. The logic of the codec manager knows how to handle that.
We are only declaring a single Image codec in this config, because there is a single model to load for images. If we have LegacySurveyImage (and HSCImage) the codec manager would load the codec 2 times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it here to fix an issue that occurred when we were running manager.encode(input, "tok_image"), and tok_image was only accessible through LegacySurveyImage. Then I saw it was not the case anymore that we could call the encode with a str instead of a modality.
Should I thus revert it back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted, but see comment to confirm it was the initial intent.
Summary
Testing
ruff check .pytest -q(fails: ProxyError when attempting to download models)