Skip to content

Conversation

@EiffL
Copy link
Contributor

@EiffL EiffL commented May 25, 2025

Summary

  • document the internal code architecture
  • list new architecture page in the docs index

Testing

  • ruff check .
  • pytest -q (fails: ProxyError when attempting to download models)

Copy link
Contributor Author

@EiffL EiffL left a 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.

"class": ImageCodec,
"repo_id": "polymathic-ai/aion-image-codec",
},
LegacySurveyImage: {
Copy link
Contributor Author

@EiffL EiffL May 26, 2025

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.

Copy link
Collaborator

@LTMeyer LTMeyer May 26, 2025

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?

Copy link
Collaborator

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.

@LTMeyer LTMeyer merged commit 56cdc59 into main May 26, 2025
3 checks passed
@LTMeyer LTMeyer deleted the codex/setup-thorough-documentation-with-theme-customization branch May 26, 2025 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants