sinzlab / mei

Visualize features cells are responsive to via gradient ascent.
MIT License
6 stars 8 forks source link

[fix] prevent cached model from being overwritten [add] pass on **kwargs to load_model function #17

Closed MaxFBurg closed 7 months ago

MaxFBurg commented 2 years ago

This PR makes the cached ModelLoader more robust preventing cached models from accidentally being overwritten. This is achieved by always returning a deepcopy instead of a reference to the cached model.

MaxFBurg commented 2 years ago

@cblessing24 @KonstantinWilleke

Black: I applied black before committing, using black --version: black, 21.12b0 (compiled: no) and -l 120 for the line length.

What is the 2nd check about?

christoph-blessing commented 2 years ago

18 needs to be merged for the status checks to pass.

MaxFBurg commented 8 months ago

Now additionally includes a change to pass **kwargs to the TrainedModel load_model function for more flexibility. @KonstantinWilleke could you please have a look and approve if happy? (I extensively tested this PR)

christoph-blessing commented 8 months ago

Why do you want to pass **kwargs? The provided model table doesn't accept any additional arguments.

MaxFBurg commented 8 months ago

Why do you want to pass **kwargs? The provided model table doesn't accept any additional arguments.

I will open a PR to nnfabrik soon that introduces kwargs (I use them in my local version). On that note, I think the model cache actually would fit better to the nnfabrik repo; maybe something to be considered for the hackathon.

christoph-blessing commented 8 months ago

I see. **kwargs is convenient but it obscures which arguments are actually being passed which makes it more difficult to reason about the code. I am wondering if your use case could be accomplished by modifying the load_model method on the table using partialmethod instead of **kwargs?