search

TeamCOMPAS / COMPAS

COMPAS rapid binary population synthesis code
http://compas.science
MIT License
64 stars 66 forks source link

Nonsense key name in COMPAS_Output.h5 generated by detailed evolution compasConfigDemo.yaml #901

Closed themikelau closed 1 year ago

themikelau commented 1 year ago

Describe the bug Instead of naming the system parameters key "BSE_System_Parameters", running examples/methods_paper_plots/detailed_evolution/runSubmitDemo.py produces COMPAS_Output.h5 with the key name "-1". All other keys (CE, RLOF, DCO, SN) are unaffected. Here is the output from h5view.py:

Summary of HDF5 file /home/mike/COMPAS/misc/examples/methods_paper_plots/detailed_evolution/COMPAS_Output/COMPAS_Output.h5
==========================================================================================================================

File size    : 0.0082 GB
Last modified: 2023-02-01 16:10:01.874454

COMPAS Filename              Columns   Entries   Unique SEEDs
--------------------------   -------   -------   ------------
Run_Details                      434         1
1                                171         2              2
BSE_Common_Envelopes              74         2              2
BSE_Double_Compact_Objects        13         2              2
BSE_RLOF                          39         4              2
BSE_Supernovae                    34         4              2

This is suspected to be an issue specific to compasConfigDemo.yaml being outdated, as opposed to COMPAS itself. Because just running ./COMPAS doens't result in this issue (v02.34.04). This issue is also averted by commenting out the line --logfile-system-parameters: -1 in compasConfigDemo.yaml.

To Reproduce Steps to reproduce the behavior:

  1. cd $COMPAS_ROOT_DIR/utils/examples/methods_paper_plots/detailed_evolution
  2. python runSubmitDemo.py
  3. Use h5view.py to look at the keys of COMPAS_Output/COMPAS_Output.h5
ilyamandel commented 1 year ago

Agree with the issue; since @jeffriley is the master of logging, I wonder whether he'd be willing to take a look?

jeffriley commented 1 year ago

@themikelau @ilyamandel It's not a nonsense filename :-) "1" (and "-1") is a valid Unix filename (and you can see from the h5view output that it has all the data in it) - it's just the we weren't expecting to see it here.

COMPAS is doing exactly what we told it to do. This line in the yaml file

--logfile-system-parameters: -1

instructs COMPAS to create the system parameters file (SSE or BSE depending upon the mode) with the name "-1". See the documentation for the option --logfile-system-parameters here:

https://compas.readthedocs.io/en/latest/pages/User%20guide/Program%20options/program-options-list-defaults.html

If you execute the following command:

./compas -n10 --logfile-system-parameters -1 --logfile-type csv

you will find that COMPAS creates a file named "-1.csv" that you can open with MS excel (or similar) and see that it contains the system parameters output.

That said, there is a problem, and I think it is my fault. When I added the record type, I added the following new options:

--logfile-common-envelopes-record-types
--logfile-detailed-output-record-types
--logfile-double-compact-objects-record-types
--logfile-pulsar-evolution-record-types
--logfile-rlof-parameters-record-types
--logfile-supernovae-record-types
--logfile-system-parameters-record-types

A value of "-1" for any of those options indicates that all record type should be written to that file (see the appropriate option in the online documentation here 

https://compas.readthedocs.io/en/latest/pages/User%20guide/Program%20options/program-options-list-defaults.html

Logfile record types are fully described for the 

--logfile-common-envelopes-record-types

option - other options refer back to there for the full description.

When I put the new options in the yaml file it looks like I copy-and-pasted from the code to get the option names - it seems my copy and paste was not quite right (orthat I didn't clean it up properly)...

The yaml file contains this:

numericalChoices:
    ### LOGISTICS
    --debug-level: 0
    --logfile-common-envelopes-record-types: -1
    --logfile-detailed-output-record-types: -1
    --logfile-double-compact-objects-record-types: -1
    --logfile-pulsar-evolution-record-types: -1
    --logfile-rlof-parameters-record-types: -1
    --logfile-supernovae-record-types: -1
    --logfile-switch-log: -1
    --logfile-system-parameters: -1
    --grid-lines-to-process: 
    ...
    ...

but it should contain this:

The yaml file contains this:

numericalChoices:
    ### LOGISTICS
    --debug-level: 0
    --logfile-common-envelopes-record-types: -1
    --logfile-detailed-output-record-types: -1
    --logfile-double-compact-objects-record-types: -1
    --logfile-pulsar-evolution-record-types: -1
    --logfile-rlof-parameters-record-types: -1
    --logfile-supernovae-record-types: -1
    --logfile-system-parameters-record-types: -1
    --grid-lines-to-process: 
    ...
    ...

(note that the '--logfile-switch-log: -1' record was removed, and the '--logfile-system-parameters: -1' line was modified)

It looks like whatever processes the yaml file successfully stripped the "-" from "-1", but that just left the filename "1".

I will fix this when I'm feeling a little better - possibly tomorrow (or someone else can do it if they want it done sooner).

If we want we could implement rules for COMPAS output file names (i.e. must start with alphabetic character, etc.), but for now COMPAS is working correctly.