search

kerrj / lerf

Code for LERF: Language Embedded Radiance Fields
https://www.lerf.io/
MIT License
668 stars 65 forks source link

fixed the bug when there are multiple query phrases #16

Closed shengjie-lin closed 1 year ago

shengjie-lin commented 1 year ago

The second time get_max_across method is called, its preset_scales parameter gets used. When there are multiple query phrases, preset_scales[i] should be the scale to be used for the i-th phrase. However, the current implementation will search again for the "best" scale for every phrase for all candidates in preset_scales. This will result in unintended behavior, and cause artifacts in the rendered relevancy map as below. artifact This pull request fixes this issue by making sure that when preset_scales is not None, preset_scales[i] only applies to the corresponding phrase.

kerrj commented 1 year ago

Looks good to me!, @chungmin99 could you take a look at the cache path changes for dataloaders?

kerrj commented 1 year ago

Thanks for the PR :)

shengjie-lin commented 1 year ago

Thanks for the reply! To be fair, changing the cache path is not necessary (lerf/data/lerf_datamanager.py#L82). However, explictly creating the cache_dir is necessary (lerf/data/utils/feature_dataloader.py#L45), otherwise nothing gets saved if cache_dir does not exist already.

chungmin99 commented 1 year ago

The intention behind the original cache_dir = f"outputs/{self.config.dataparser.data.name} was to store each scene's clip/dino data separately (e.g., outputs/teatime, outputs/kitchen, ...). Would it possible for you to revert that change?

shengjie-lin commented 1 year ago

@chungmin99 Sure, no problem. But just in case my point is missed, even with my modified code cache_dir = self.config.dataparser.data / 'lerf_cache', each scene's clip/dino data are still stored separately, but alongside their training data. Also, as I said earlier, explicitly creating the cache_dir is necessary. Consider the case when we do ns-train lerf and do not pass --output-dir to it, nerfstudio will create the default outputs/[scene_name] dir for us. That's why you might not encounter this problem. However, when we do pass a different --output-dir, there won't be outputs/[scene_name]. So we need to make the dir when needed. I created another PR just containing the necessary changes. Closed this one.