seoulsky-field / CXRAIL-dev

CXRAIL-dev
MIT License
7 stars 0 forks source link

Hotfix: Did not change output config.yaml #86

Closed seoulsky-field closed 1 year ago

seoulsky-field commented 1 year ago

What

Why


How

jieonh commented 1 year ago

As far as I know,hydra/config.yaml only logs the currently saved config list on the hydra, so the override with the cli command is not reflected. Would you check the hydra/overrides.yaml? I guess the overridden changes are likely to be saved there but I'm not so sure..!

seoulsky-field commented 1 year ago

@jieonh Thanks to reply. I checked it, it has empty value like a below image. 스크린샷 2023-01-27 오후 4 53 53

chrstnkgn commented 1 year ago

Could you try python inference.py +Dataset.root_path="/home/" ? It might work but I'm not so sure - assuming there might be some problem in default settings but I should delve into inference code and test.yaml in order to understand it. I'll look at it and comment later.

jieonh commented 1 year ago

@seoulsky-field I've run it with the same command(python main.py Dataset.train_size=1000 Dataset.root_path='/home/'), found logs in the hydra/overrides.yaml !

image

chrstnkgn commented 1 year ago

Ok I got it. First thing first, you might also want to try python inference.py CheXpert.root_path="/home/"

The default settings in test.yaml file is sown as follows. image Which means that this file has multiple defaults and we assign the actual default in the code itself. If we are using CheXpert dataset and assign the dataset default as so, the hydra will interpret the input as CheXpert, not Dataset.

seoulsky-field commented 1 year ago

@chrstnkgn Thank you for your comment. I tried it but got a same error when I typed python inference.py +Dataset.root_path="/home/". Also, I already tried python inference.py CheXpert.root_path="/home/", I checked it would work well. (It's just guess because server change, test.csv format is changed so that it cannot work.)

However, the user should type twice (when execute main and inference), I thought it would be changed.

jieonh commented 1 year ago

I guess it might be better to modify the inference.py code to also reflect the overridden content

chrstnkgn commented 1 year ago

Great! We could simply change the yaml config temporarly as this error is caused by the change of our directory paths.

seoulsky-field commented 1 year ago

@jieonh Thanks to double check! I'm really curious about how hydra save overrides. I execute same commands and after that, I checked overrides.yaml, but this content existed. 스크린샷 2023-01-27 오후 5 12 29

I correctly use same commands but I don't know why different contents in overrides.yaml.. 🧐

jieonh commented 1 year ago

@seoulsky-field I noticed that the path of your override.yaml file is somewhat different, and found out that two different logs are being saved in your working dir(logs, output). I wonder where the output logging file came from because I don't have that one. Anyway, if you check the other logs in the logsfile, you can see that the contents are the same as my execution result!

image

image

seoulsky-field commented 1 year ago

@jieonh Thanks to check! After you said, I checked the logs folder and found config.yaml in their. Also, that config.yaml was revised correctly.

However, I have not gotten the reason that made path error. Do you have any doubts?