Skip to content
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

Tiles - improved support for cached data loading #67

Open
jacksonjacobs1 opened this issue Feb 5, 2025 · 7 comments
Open

Tiles - improved support for cached data loading #67

jacksonjacobs1 opened this issue Feb 5, 2025 · 7 comments
Assignees
Labels
enhancement New feature or request v2.0 Pertains to the v2.0 branch

Comments

@jacksonjacobs1
Copy link
Collaborator

Description

Caching and cache invalidation of ground truth tiles is extremely important for the DL component.

  1. Caching of a ground truth tile will occur each time a tile mask is constructed.
  2. Cache invalidation will occur each time a tile's annotations are added, removed, or updated

Implementation notes

The tile table will need to be changed so that updates pertaining to ground truths can be distinguished from updates pertaining to predictions:

Current schema:

    # primary key
    id = Column(Integer, primary_key=True, autoincrement=True)

    # foreign keys
    annotation_class_id = Column(Integer, ForeignKey('annotation_class.id'), nullable=False)
    image_id = Column(Integer, ForeignKey('image.id'), nullable=False)
    tile_id = Column(Integer, nullable=False)

    # columns
    seen = Column(Integer, nullable=False, default=0)
    hasgt = Column(Boolean, nullable=False, default=False)
    datetime = Column(DateTime, server_default=db.func.now())

New Schema:

    # primary key
    id = Column(Integer, primary_key=True, autoincrement=True)

    # foreign keys
    annotation_class_id = Column(Integer, ForeignKey('annotation_class.id'), nullable=False)
    image_id = Column(Integer, ForeignKey('image.id'), nullable=False)
    tile_id = Column(Integer, nullable=False)

    # columns
    seen = Column(Integer, nullable=False, default=0)
    hasgt = Column(Boolean, nullable=False, default=False)
    gt_last_updated = Column(DateTime, server_default=db.func.now())
    pred_last_updated = Column(DateTime, server_default=db.func.now())

Alternative New Schema:

    # primary key
    id = Column(Integer, primary_key=True, autoincrement=True)

    # foreign keys
    annotation_class_id = Column(Integer, ForeignKey('annotation_class.id'), nullable=False)
    image_id = Column(Integer, ForeignKey('image.id'), nullable=False)
    tile_id = Column(Integer, nullable=False)

    # columns
    is_processing = Column(Boolean, nullable=False, default=False)
    gt_last_updated = Column(DateTime, server_default=db.func.now(), nullable=True)        # where null means no gt annotations exist
    pred_last_updated = Column(DateTime, server_default=db.func.now(), nullable=True)   # where null means no pred annotations exist
@jacksonjacobs1 jacksonjacobs1 added enhancement New feature or request v2.0 Pertains to the v2.0 branch labels Feb 5, 2025
@jacksonjacobs1 jacksonjacobs1 self-assigned this Feb 5, 2025
@choosehappy
Copy link
Owner

alternative seems more attractive to me --- i also wonder if is_processing should just be "processing_status" and be an integer that is updated as the tile moves through different "phases"? "submitted" "completed"? or... does the binary + the date column address all situations? is there a case where it is processing, but predictions already exist, that leave us in an unknown state?

@jacksonjacobs1
Copy link
Collaborator Author

If is_processing is True, or 1 or whatever, this should always mean no predictions are saved to that tile. That's because we decided that every time annotations are refreshed (the user wants updated predictions), we do 2 things:

  1. Drop all annotations tables for the refreshed class
  2. Reset all tiles for that class - set pred_last_updated to null, indicating that no predicted annotations exist for the tile.

What about making is_processing instead store a reference to the prediction job? I don't think we're actually tracking the jobs anywhere currently? Then null would indicate that a prediction job has not yet been initiated, and we can use the job ref to check the status of the job in live time.

@choosehappy
Copy link
Owner

A few thoughts here:

  1. if the user hits the refresh button, that is certainly one a big change - and tables should be dropped. totally agree
  2. however, if the user moves away from a tile, and then comes back to that tile e.g., 10 minutes later, i feel like the predictions should be updated unless they hit the "pause" button we spoke about. This implies something like a tile level manipulation, wherein now that we have a tileid, we can easily simply delete the annotations associated with that tile and trigger another prediction job.

otherwise its not clear to me what the "refresh" functionality would look like or how that would be engaged?

i think this basically changed when we decided to have rolling-updates, and now the "datetime" column is what determines "sufficient freshness"

a question would be when to delete exactly. i would say we probably delete the previous annotations right before inserting new ones, as opposed to deleting, then running inference (and hoping that it doesn't crash)

  1. given the way the DL is setup, there is no inference job : ) the inference is going to take place in the perpetual training loop job. take a look at that training.py code and you'll get a better idea of why/how these are tightly coupled

@jacksonjacobs1
Copy link
Collaborator Author

Ahhhh okay. This all makes sense to me.

a question would be when to delete exactly. i would say we probably delete the previous annotations right before inserting new ones, as opposed to deleting, then running inference (and hoping that it doesn't crash)

Had to mull over this for a bit, but I like it! Much smoother than preemptively deleting pred annotations. We'll just have to keep in mind that a tile can now have an additional possible state (is_processing = 1 and annotations exist for the tile in the database).

I'm also okay with renaming is_processing to pred_processing_stage as an unsigned int as this probably gives us the most flexibility going forward. For now though, the only possible values should be 0 and 1, right?

@choosehappy
Copy link
Owner

(is_processing = 1 and annotations exist for the tile in the database)

indeed, what would the front end do in this situation? show what was available in the backend or if is_proc = 1 show nothing? what happens when the proc finishes, does the front end refresh that tile with the "latest" results? otherwise we have a weird sitaution where the front end could have predicted polygons which don't exist in the backend DB anymore ;-)

stage

i would say to make it a signed short int, either way its going to take a 1 byte, and i don't think we'll get up to >127 states, however being able to have a -1 state is usually pretty interesting (e.g., indicates an error has taken place)

probably 4 states? -1 (Fail), 0 (not even viewed, default, same/different as Null?), 1 (processing), 2 (done)

and the datetime indicates when it went into that state?

@jacksonjacobs1
Copy link
Collaborator Author

indeed, what would the front end do in this situation? show what was available in the backend or if is_proc = 1 show nothing? what happens when the proc finishes, does the front end refresh that tile with the "latest" results? otherwise we have a weird sitaution where the front end could have predicted polygons which don't exist in the backend DB anymore ;-)

Yeahh... end of the day we don't want this choice to create more headaches for us in the future. Less possible tile states probably means less headaches.

If we delete annotation before running inference (and remove the tile feature from the viewport if it exists), and the inference fails, we can just set the tile back to the unseen state. Using it's current polling implementation, the client will recognize this and create a new inference job within half a second or so.

@choosehappy
Copy link
Owner

this seems fine, but i also feel like there is increasing complexity - do we have something like (or should we make sometihng like) a workflow diagram to better understand what is happening in what places and what the different status variables (datetime + status) should look like to see if everything fits together nicely?

here for example there seems to be a case wherein the UI gets the predictions and displays them, after X period of time, a new models is available and thus new predictions should take place. with your proposal, this would result in the preds being deleted from the backend -- would we immediately clear the front end visible ones? -- so then there would be a "blank" spot while the backend generates the new annotations, and then they appear? so there is a time period where there are no predictions visible? is that "okay"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v2.0 Pertains to the v2.0 branch
Projects
None yet
Development

No branches or pull requests

2 participants