proposal: have enzo exit on unrecognized parameter

[Britton Smith]

Hi everyone,

I recently submitted a rather minor pull request to fix a typo in the writing of one of Enzo's parameter.  Have a look here for that:

https://bitbucket.org/enzo/enzo-dev/pull-requests/335/fixing-a-typo-and-removing-an-unused/diff

If you were running with this bug, you may have noticed the following line fly by at near light speed when you started a simulation:

warning: the following parameter line was not interpreted

MHDPowellSource          = 0

While this parameter isn't used by most of us, if you had a typo of a critical parameter in your simulation, this is the only warning you would see.  Because of the number of things printed at startup, you would probably miss it, and the simulation would go on running.

My proposal is to have an unrecognized parameter trigger an ENZO_FAIL with an explanatory message.  My personal opinion is that I would rather have an incorrectly configured simulation stop immediately so I can fix it instead of running to the end and wasting CPU hours.

Can we get a simple +-1 on changing this behavior?  If there is support, I will add it to the above PR.

Britton

[John Wise]

+1

I've had typos in parameters ruin simulations before.

John

> --
> You received this message because you are subscribed to the Google
> Groups "enzo-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to enzo-dev+u...@googlegroups.com
> <mailto:enzo-dev+u...@googlegroups.com>.
> To post to this group, send email to enzo...@googlegroups.com
> <mailto:enzo...@googlegroups.com>.
> Visit this group at https://groups.google.com/group/enzo-dev.
> For more options, visit https://groups.google.com/d/optout.

--
John Wise
Associate Professor of Physics
Center for Relativistic Astrophysics, Georgia Tech
http://cosmo.gatech.edu

[Andrew Emerick]

+1 from me. As one who has wasted CPU hours for this exact reason I fully support this. It would be useful to code it such that it fails only after running through the entire parameter file; that way the user knows all of the unknown parameters in one shot. 

Andrew 

--

You received this message because you are subscribed to the Google Groups "enzo-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to enzo-dev+u...@googlegroups.com.
To post to this group, send email to enzo...@googlegroups.com.

Visit this group at https://groups.google.com/group/enzo-dev.
For more options, visit https://groups.google.com/d/optout.

--

---

NSF Graduate Research Fellow

Columbia University

Department of Astronomy

[Brian O'Shea]

+1.  This is a great idea and will save a lot of headaches for new and old users alike.

--

[Greg Bryan]

+1

[John Regan]

+1

[Britton Smith]

This sounds like consensus enough for me (sorry to those west of Indiana who are still in bed).  I've updated the PR.

[Christine Simpson]

+1  I would also add, that it may also be useful to have some kind of warning or failure if a parameter is defined twice in a parameter file.  I know more than one of us has made this mistake and it can be very hard to discover.

[Brian O'Shea]

http://i.imgur.com/xuzX5.gif

[Brian O'Shea]

Hi Christine,

That's a great idea, and I've been bitten by that one before as well.  I think it's substantially harder to implement than a simple ENZO_FAIL, though.  For each parameter, you'd have to check to see if its value was no longer the default, and throw an ENZO_FAIL.  It'd be a lot of lines of code given the way ReadParameterFile is currently structured.  (I think.)  It'd be a good contribution to the codebase if you're interested in doing it!

--Brian

[Britton Smith]

The only other way I could see implementing that would be to create a dictionary-like object, store an entry for every parameter that has been read in, and then fail if the dictionary entry already exists.  I am not very familiar with this type of structure in C++, but if someone wants to try it, this might be a good strategy.

[David Collins]

I'm +1 on this.

Can I additionally request a command line option to Enzo that allows one to just parse the parameter file?  That way I can check my parameter file before it sits in the queue.

I'm pretty sure it would be easy, I'll look into that next week if no one else has a free hand.  

I like Christine's idea as well.

d.

-- Sent from a computer.

[Brian O'Shea]

Hi Dave, all,

First: I love the idea of a command-line option to parse the parameter files.  That seems very helpful.

Second, and more urgently: the test suite has highlighted that there are some challenges with the PR as it stands.  There are a *ton* of parameters  that are in the various test problems that appear to be related to the MHD (MHD2DProblemType, SlopeLimiter, DualEnergyMethod, ...) that are not read in via ReadParameterFile.  Some grepping of the source code indicates that a lot of these parameters have been changed to reflect their MHD origins (DualEnergyMethod has been replaced by MHDCTDualEnergyMethod, SlopeLimiter by MHDCTSlopeLimiter, etc.), which suggests that we need to update the parameter files for these test problems.  But, there are also a bunch of problem types (mostly coming from Tom Abel and Peng Wang's MHD) that have parameter names like SphereRadius which need to be tweaked.  We can fix this, but it'll involve a lot of changes to the source code, the parameter files, and the documentation.  I'm generally OK with that, but it's a pretty big commitment of time on somebody's part.  Perhaps this is something to consider as a project for the next Enzo developer meeting, rather than as a part of this PR.  What do people think?

--Brian

[Brian O'Shea]

Just following up on this - what do people think about making the changes required to have Enzo fail on an unrecognized parameter?  Is it worth doing now, or should it be added as an agenda item for the next Enzo workshop?  This will be the *most* work for people familiar with MHD, but I'd guess it's probably not more than ~10 developer-hours in total effort, unless something goes horribly awry.

--Brian

[Matthew Turk]

I'm +1 on the idea, -1 on doing it before a workshop.

[Britton Smith]

This sounds like a great workshop activity.  I would be fine with waiting until then to put this in place.  If that's what we decide, I would be happy to strip the last changeset from my PR so we can at least get the typo fixed.

[Brian O'Shea]

OK, I agree with both of you.  Let's hold off on putting this particular feature in place until the next workshop!