bd2kccd / causal-cmd

16 stars 8 forks source link

txt output file is not created with prefix when json-graph output enabled after 2nd time execution #78

Closed yasu-sh closed 1 year ago

yasu-sh commented 1 year ago

Hi,

I doubt if the existing json file suppreess the .txt file output when json-graph with prefix. Could you fix this issue without workarounds? it makes better.

I guess the process is as below:

  1. determines prefix-filename
  2. checks existing json-file
  3. if exist, only creates json-file(same filename), but not creates txt-file.
  4. as a result, "prefix".txt file disappear

So far, I have used the workaround:

  1. deletes json and txt files
  2. executes causal-cmd
  3. rename json and txt with current date-time for next execution
PS > java -jar causal-cmd-1.4.1-jar-with-dependencies.jar --algorithm fges --data-type discrete --delimiter comma --score bdeu-score --json-graph --dataset asia.csv --prefix c-tetrad
PS > dir c-tetrad*
Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
-a----        2022/12/07     10:04           2044 c-tetrad.txt
-a----        2022/12/07     10:04          33109 c-tetrad_graph.json

PS > java -jar causal-cmd-1.4.1-jar-with-dependencies.jar --algorithm fges --data-type discrete --delimiter comma --score bdeu-score --json-graph --dataset asia.csv --prefix c-tetrad
PS > dir c-tetrad*
Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
-a----        2022/12/07     10:05          33123 c-tetrad_graph.json

PS > del c-tetrad*
PS > java -jar causal-cmd-1.4.1-jar-with-dependencies.jar --algorithm fges --data-type discrete --delimiter comma --score bdeu-score --json-graph --dataset asia.csv --prefix c-tetrad
PS > dir c-tetrad*
Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
-a----        2022/12/07     10:05           2045 c-tetrad.txt
-a----        2022/12/07     10:05          33123 c-tetrad_graph.json

asia.csv data: asia.csv (created from bnlearn, data(asia)) causal-cmd version 1.4.1, downloaded from https://s01.oss.sonatype.org/content/repositories/releases/io/github/cmu-phil/causal-cmd/1.4.1/

java -version openjdk version "1.8.0_342" OpenJDK Runtime Environment Corretto-8.342.07.3 (build 1.8.0_342-b07) OpenJDK 64-Bit Server VM Corretto-8.342.07.3 (build 25.342-b07, mixed mode)

kvb2univpitt commented 1 year ago

@yasu-sh Ah..I do see the bug! The .txt file should not be deleted on the second run. Ideally, if you run it the second time, it will overwrite the existing result files. Do you to have it keep the original result files? If so, I can append a number on the new result files.

yasu-sh commented 1 year ago

@kvb2univpitt Thanks for your quick response and checks.

Do you to have it keep the original result files?

No. I do not have to keep the original result files. It is suitable and valid for me that the causal-cmd overwrites the result files if I specifiy the filename. I think most of users do not want to check the appeded new numbers in filenames every time in this case.

kvb2univpitt commented 1 year ago

@yasu-sh It's now fixed in the new release causal-cmd-1.4.2.

yasu-sh commented 1 year ago

@kvb2univpitt What's a lovely action! I have cloned the repository and built it. it is solved. I remembered the meaning of "prefix", out of consideration of "prefix" - "root" - "suffix" relationship.

fges_(numericdatetime).txt - fges(numeric_datetime)_graph.json ... "datetime" is root? (prefix).txt - (prefix)_graph.json ...

But my intention has been already accepted. Related arguments names may be changed if developes like!

I checked the behaviors and building at:

kvb2univpitt commented 1 year ago

@yasu-sh I agree. It's a bit confusing with the --prefix flag. Initially, it was only meant for JSON graph output. But, I have extended to the text output. I should fix it so that the text output should look like this <prefix>_out.txt when ---prefix is used, otherwise <algorithm_name>_<numeric timestamp>_out.txt. Any suggestions? By the way, I really appreciate your help in posting bugs. :)

yasu-sh commented 1 year ago

@kvb2univpitt <prefix>_out.txt would be reasonable for import programs to promise always latest output files. Intially users use interactive execution, then using the results from outside, for example converting to diagrams(node - edges). I feel comfortable to name <prefix>_out.txt since PrintStream in defined in the code is subclass of OutputStream in java.io class.

kvb2univpitt commented 1 year ago

@yasu-sh Sounds good. Instead of making another release, I'll just add it to the 1.4.2 release. I'll reopen this issue. You can close it once you verify that it's working.

yasu-sh commented 1 year ago

@kvb2univpitt I see. I'll have a look after updated code is commited. Thanks for your kindness.

kvb2univpitt commented 1 year ago

@yasu-sh OK. It's fixed. I also fixed the help description for the prefix command-line option so that people know what the prefix is replacing:

Replace the default output filename prefix in the format of <algorithm>_<numeric timestamp>.
yasu-sh commented 1 year ago

@kvb2univpitt It works! Thanks a lot. I confirmed as below.