-
Notifications
You must be signed in to change notification settings - Fork 3
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
Bootstrap the library #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cwognum
added
the
feature
Annotates any PR that adds new features; Used in the release process
label
Apr 26, 2024
Closed
5 tasks
* dev refactor * fix test * fix tests * remove unused code * refactor repo name * Reviewed @zhu0619's changes. Switched to jinja2 for HTML broadcaster * Fixed bug in LoggerBroadcaster * Fixed release CICD * Fix CICD --------- Co-authored-by: cwognum <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As @zhu0619 and me were working through polaris-hub/polaris#98, we realized that the
curation
module within the Polaris library really should be a stand-alone package. It is independent from the broader Polaris codebase. Up until now this was fine, but as we go public with Polaris, we will need to be more careful about our release strategy, which would unnecessarily slow down the development of the curation module.This PR moves the
curation
module into its own package, temporarily namedalchemy
.Refactoring
As part of this PR, we decided to refactor the code base to be better setup for future maintenance.
In summary:
Curator
class. This class specifies the curation process as a number of steps. It can be serialized to be easily saved to and loaded from JSON. The goal with this is to make the process more easily reproducible.BaseAction
objects.curation.functional
module, inspired bytorch.nn.functional
, to easily use any of the curation steps in isolation, outside of the object-based approach.CurationReport
, which is produced by theCurator
and holds relevant information about the curation process (for now just logs and images) and can be exported to different formats throughBroadcasters
, such as theLoggerBroadcaster
andHTMLBroadcaster
.visualization
module and refactored the visualizations to be more general.The object-based API now looks like:
You could do something similar with the functional API, e.g.:
Relation to Polaris Recipes
Now that we have this system, I'm thinking we should require every
polaris-recipe
to provide the serializedCurator
object as JSON. That way, everyone that has access to the base dataset could reproduce the process with the CLI we added.TODO
(Note: We don't need to do all these things in this PR, but just for future reference)
Add more documentation (bit of an empty shell right now)