search

openscm / pymagicc

Python wrapper for the simple climate model MAGICC
https://pymagicc.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
39 stars 27 forks source link

f90nml 1.3 #325

Open znicholls opened 3 years ago

znicholls commented 3 years ago

Describe the bug

f90nml 1.3's string splitting (https://github.com/marshallward/f90nml/pull/120/files) can cause MAGICC running to break if you try to write a long list of string arrays. For example, f90nml will now write output like the below, which can't be parsed by MAGICC (because the commas are in the wrong place).

out_dynamic_vars = 'DAT_SURFACE_TEMP', 'DAT_SURFACE_MIXEDLAYERTEMP',
'DAT_TOTAL_INCLVOLCANIC_ERF', 'DAT_TOTAER_DIR_ERF', 'DAT_CO2_ERF', 'DAT_,  # this line is wrong
CO2PF_EMIS', 'DAT_CH4PF_EMIS'

Failing Test

TBD

Expected behaviour

The output should be like the below

out_dynamic_vars = 'DAT_SURFACE_TEMP', 'DAT_SURFACE_MIXEDLAYERTEMP',
'DAT_TOTAL_INCLVOLCANIC_ERF', 'DAT_TOTAER_DIR_ERF', 'DAT_CO2_ERF', 
'DAT_CO2PF_EMIS', 'DAT_CH4PF_EMIS'

Screenshots

If applicable, add screenshots to help explain your problem.

System (please complete the following information):

Additional context

It's unclear to me if this is a pymagicc bug or an f90nml bug. One for further investigation.

znicholls commented 3 years ago

@lewisjared and @rgieseke fyi, workaround is just pin f90nml==1.2

lewisjared commented 3 years ago

I opened https://github.com/marshallward/f90nml/issues/131 the other day and a fix is in the works. I've been pinning 1.2 locally to resolve this

znicholls commented 3 years ago

Legend

marshallward commented 3 years ago

I've updated PyPI to 1.3.1 which fixes the errant comma and also defaults back to the old behaviour. I've also added an array string test so this should be detected in the future.

rgieseke commented 3 years ago

Thank you so much for your work! :-) Used by NASA and in IPCC climate model pipelines ... quite a userbase!

znicholls commented 3 years ago

Legend thanks @marshallward !