Ramses read namelist #2347

cphyc · 2019-10-18T14:28:17Z

PR Summary

This adds the automatic loading of the namelist.txt file which contains the parameter file RAMSES used to produce the output.
This relies on the package f90nml being installed (for the tests). If the package is absent, ds.parameters['namelist'] contains an error message.

PR Checklist

Code passes flake8 checker
Adds a test for any bugs fixed. Adds tests for new features.

matthewturk

LGTM!

matthewturk · 2019-10-18T14:45:21Z

yt/frontends/ramses/data_structures.py

+                with open(namelist_file, 'r') as f:
+                    nml = f90nml.read(f)
+            except ImportError as e:
+                nml = "An error occurred when reading the namelist: %s" % str(e)


If we make this an exception of some type, we might be more easily able to determine if it's the success or not, with an isinstance.

I am not sure I understand your comment.

However, the reason I settled for this is that reading namelist.nml is not required to properly load the simulation, so I was trying to avoid throwing errors at the user.

NBD, just thought if we had an object we could check if it read correctly without doing a string comparison. I agree about not throwing errors!

I am still confused ; where do you see a string comparison? Here I am storing the content of the namelist in ds.parameters['namelist']. If we're missing the f90nml package, I store the error message instead.

That is probably not the best solution though.

Sorry, I meant -- let's pretend I wanted to know if the namelist had been parsed. I could do that with an isinstance or I could check the contents of the string, right? Or is it only going to be an instance of str if it succeeds?

There are three cases:

the file does not exists: ds.parameters['namelist'] is left unitialized,

the file exists and f90nml is missing: ds.parameters['namelist'] then contains the error message as a string,

the file exists and f90nml is installed: ds.parameters['namelist'] is a dict-like structure with the content of the namelist.

Any idea to make it better is welcome!

So right now if I wanted to see if the namelist was parsed, I wouldn't have a proper way of doing it except checking isinstance(ds.parameters['namelist'], str).

matthewturk · 2019-10-18T15:13:11Z

Nah, this looks great to me!

…

On Fri, Oct 18, 2019 at 10:12 AM Corentin Cadiou ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In yt/frontends/ramses/data_structures.py <#2347 (comment)>: > @@ -582,6 +583,21 @@ def read_rhs(f, cast): if self.num_groups > 0: self.group_size = rheader['ncpu'] // self.num_groups + # Read namelist.txt file (if any) + self.read_namelist() + + def read_namelist(self): + """Read the namelist.txt file in the output folder, if present""" + namelist_file = os.path.join(self.root_folder, 'namelist.txt') + if os.path.exists(namelist_file): + try: + with open(namelist_file, 'r') as f: + nml = f90nml.read(f) + except ImportError as e: + nml = "An error occurred when reading the namelist: %s" % str(e) There are three cases: 1. the file does not exist and ds.parameters['namelist'] does not exist, 2. the file exists and f90nml is missing: ds.parameters['namelist'] then contains the error message as a string, 3. the file exists and f90nml is installed: ds.parameters['namelist'] is a dict-like structure with the content of the namelist. Any idea to make it better is welcome! — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2347?email_source=notifications&email_token=AAAVXO56RBKVNN3MKIVS7BLQPHG5RA5CNFSM4JCIGAXKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCIPASPY#discussion_r336541227>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAVXO3S5PTBKPXAAAH4IMTQPHG5RANCNFSM4JCIGAXA> .

cphyc · 2019-10-18T17:12:17Z

The tests fail because the new test assumes f90nml to be installed (which is not the case). Would it be possible to install it on jenkins?

munkm · 2019-10-18T19:08:00Z

@Xarthisius what do we need to do to get f90nml on Jenkins?

Xarthisius · 2019-10-18T19:18:43Z

The tests fail because the new test assumes f90nml to be installed (which is not the case). Would it be possible to install it on jenkins?

Since it's a python package, all you need to do is add it to yt/tests/test_requirements.txt. However, those tests should be wrapped in @requires_module("f90nml") too to avoid that situation when someone's running it locally.

munkm · 2019-10-18T19:19:40Z

yt/frontends/ramses/tests/test_outputs.py

+
+output_00080 = "output_00080/info_00080.txt"
+@requires_ds(output_00080)


Because this test uses the f90nml module I think you need the @requires_module() decorator.

Ha nice mind-meld with @Xarthisius! 😎

matthewturk

This looks ready to go to me!

EDIT: Whoops, I didn't read in detail the errors -- I don'tknow what's up, but it looks like a ValueError is being raised from the new tests. Do we need to update the dataset?

cphyc · 2019-10-21T14:30:08Z

@matthewturk I think we should rather catch the error and display it as a warning to the user. There used to be a bug in RAMSES that would write the configuration file in binary format. I'm working on it.

cphyc · 2019-10-22T14:00:08Z

@yt-fido test this please.

cphyc · 2019-10-24T11:22:15Z

The tests seem to fail for unrelated reasons, don't they?

munkm · 2019-10-25T19:42:47Z

I'm working on a fix for the 3.6 answer tests on Travis.

munkm · 2019-10-25T19:47:35Z

@cphyc if you merge or rebase with master you should get the fix I pushed for the Travis tests.

Xarthisius · 2019-10-26T00:36:05Z

@yt-fido test this please

Also add on-demand facilities for f90nml package

This happens with old version of RAMSES that produced malformed namelist.txt files

The tests were relying on output_00080 for a *correct* namelist. Changed it to a working one (ramses_new_format). The path to namelist.txt was not computed correclty, resulting in a failing test.

matthewturk approved these changes Oct 18, 2019

View reviewed changes

munkm reviewed Oct 18, 2019

View reviewed changes

matthewturk approved these changes Oct 20, 2019

View reviewed changes

cphyc added 7 commits October 27, 2019 21:19

Read namelist.txt file, if present

172a99a

Also add on-demand facilities for f90nml package

And test it

471feea

Add f90nml as test requirement

993f998

Catch ValueError on namelist.txt reading

ecd2c10

This happens with old version of RAMSES that produced malformed namelist.txt files

Test reading of malformed namelist

f80cf37

Pass the error in the warning

0fbc35f

Fix erroneous test

a528d21

The tests were relying on output_00080 for a *correct* namelist. Changed it to a working one (ramses_new_format). The path to namelist.txt was not computed correclty, resulting in a failing test.

cphyc force-pushed the RAMSES-read-namelist branch from 54c5a33 to a528d21 Compare October 27, 2019 21:19

munkm merged commit 251a491 into yt-project:master Oct 28, 2019

This was referenced Nov 25, 2019

[AMRVAC] Optionally supply parfiles to populate AMRVACDataset.parameters #2368

Closed

[AMRVAC] Parse AMRVAC's parfiles if user-provided #2369

Merged

cphyc deleted the RAMSES-read-namelist branch January 13, 2021 11:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ramses read namelist #2347

Ramses read namelist #2347

cphyc commented Oct 18, 2019 •

edited

Loading

matthewturk left a comment

matthewturk Oct 18, 2019

cphyc Oct 18, 2019

matthewturk Oct 18, 2019

cphyc Oct 18, 2019 •

edited

Loading

matthewturk Oct 18, 2019

cphyc Oct 18, 2019 •

edited

Loading

cphyc Oct 18, 2019 •

edited

Loading

matthewturk commented Oct 18, 2019 via email

cphyc commented Oct 18, 2019

munkm commented Oct 18, 2019

Xarthisius commented Oct 18, 2019 •

edited

Loading

munkm Oct 18, 2019

munkm Oct 18, 2019

matthewturk left a comment •

edited

Loading

cphyc commented Oct 21, 2019

cphyc commented Oct 22, 2019

cphyc commented Oct 24, 2019

munkm commented Oct 25, 2019

munkm commented Oct 25, 2019

Xarthisius commented Oct 26, 2019


		output_00080 = "output_00080/info_00080.txt"
		@requires_ds(output_00080)

Ramses read namelist #2347

Ramses read namelist #2347

Conversation

cphyc commented Oct 18, 2019 • edited Loading

PR Summary

PR Checklist

matthewturk left a comment

Choose a reason for hiding this comment

matthewturk Oct 18, 2019

Choose a reason for hiding this comment

cphyc Oct 18, 2019

Choose a reason for hiding this comment

matthewturk Oct 18, 2019

Choose a reason for hiding this comment

cphyc Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

matthewturk Oct 18, 2019

Choose a reason for hiding this comment

cphyc Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

cphyc Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

matthewturk commented Oct 18, 2019 via email

cphyc commented Oct 18, 2019

munkm commented Oct 18, 2019

Xarthisius commented Oct 18, 2019 • edited Loading

munkm Oct 18, 2019

Choose a reason for hiding this comment

munkm Oct 18, 2019

Choose a reason for hiding this comment

matthewturk left a comment • edited Loading

Choose a reason for hiding this comment

cphyc commented Oct 21, 2019

cphyc commented Oct 22, 2019

cphyc commented Oct 24, 2019

munkm commented Oct 25, 2019

munkm commented Oct 25, 2019

Xarthisius commented Oct 26, 2019

cphyc commented Oct 18, 2019 •

edited

Loading

cphyc Oct 18, 2019 •

edited

Loading

cphyc Oct 18, 2019 •

edited

Loading

cphyc Oct 18, 2019 •

edited

Loading

Xarthisius commented Oct 18, 2019 •

edited

Loading

matthewturk left a comment •

edited

Loading