search

interpreting-rl-behavior / interpreting-rl-behavior.github.io

Code for the site https://interpreting-rl-behavior.github.io/
Creative Commons Attribution 4.0 International
0 stars 0 forks source link

Add saliency timestep indicator #58

Closed danbraunai closed 2 years ago

danbraunai commented 2 years ago

We want it to be clear which timestep the saliency is taken from. This could be a phrase like "timesteps until saliency: 2", or some other clear indicator

danbraunai commented 2 years ago

In order to store the specified saliency timestep when running the saliency experiment, we just need to fetch the common_timesteps hyperparameter inside the saliency_experiments.py and write it to file (which can then be read by import_data.py and moved to wherever it is needed.

Since I was already given the data from running all the experiments, and this data doesn't contain the saliency timestep, I just infer it by the first timestep where the grads are all zero in import_data.py in commit #253330 train-procgen-pytorch repo.

danbraunai commented 2 years ago

As seen in the above commit, I just added a "Saliency step: X" below Saliency Type. This looks OK when saliency type is a dropdown: image

but imo adds too much vertical space when there are two radio buttons: image

Regardless, we might want a solution that is easier to understand than this. One option is to change the "Saliency Map" label: image

We could even colour code the label to match the corresponding control above it. Not sure how I feel about duplicating information in these panels though. I'm leaning towards something like what I implemented in the commit above, maybe with only using dropdowns and not radio buttons due to vertical space.

How do you feel about this stuff? One thing we might want to do when an article draft gets sent out to a few people is ask for opinions on the panel and I can make small changes based on that.

leesharkey commented 2 years ago

imo adds too much vertical space when there are two radio buttons

I don't have strong feeling on the amount of vertical space. If there's a panel where we're only showing two types of saliency, i think the extra space is worth it to display both types of saliency simultaneously as radio buttons. No strong feelings on that. But I defs prefer the extra vertical space to the changed Saliency Map Label though.

One thing we might want to do when an article draft gets sent out to a few people is ask for opinions on the panel and I can make small changes based on that.

Yeah, it'll be worth doing this, but tbh I don't expect we'll need to do many more changes, this is looking really good.