Alneos / vega

Finite element format converter
GNU General Public License v2.0
23 stars 9 forks source link

Invalid commands in NASTRAN control case does not raise error. #26

Closed ThomasAbballe closed 4 years ago

ThomasAbballe commented 5 years ago

Hi all,

In one of my model, the case control command P2G is not recognized by VEGA (which is problematic for me, but that's another thing). Problem is, it does not raise any errors or warnings. After some tests, the same bug applies to every "wrong" commands in the case control part of the DAT file: unknown commands are quietly ignored.

VEGA must raise an error when it read unknown commands in the case control part of the DAT file.

Cheers, Thomas

ldallolio commented 4 years ago

Hi Thomas, I just added commit 9462f35b415dfd080917c53fc68bb616a284520a to add a warning and a unitcase for parameters that are not correctly formed. Still I am not sure that all cases are covered. If a parameter is "well formed" (meaning it has a name, '=' symbol, a value) then it is added into a context (= map) and later on, the parser might use it (or not). We could consider keeping track of "unused" parameters and writing warnings at the end of parsing, like we already do at the end of writing for objects. So, I let you decide if this ticket should be closed now or not. By the way, I also had a look at P2G: I would say that it is feasible, not exactly easy but feasible. An example in Nastran would really help, but I could also write one if needed. Cheers, Luca

ThomasAbballe commented 4 years ago

It's a good start, but not enough, in my opinion. This version still does not show any warning saying it does not support P2G, which is a problem. Writing which parameters are not used is a good idea, but I'm not sure it's in the right direction. I'd prefer we control the key during the parsing, like we do for the BULK command. It's a bit more tedious (we need a map of ignored parameters for example), but I think it's more robust, and well, more consistent with the rest of the parser.

ldallolio commented 4 years ago

Hi Thomas, I did some more work on this following your advices, committed as 3b212464e235ce769e00dc3c7e24f783611bec54 I think that this should do it, in fact I needed to have a list not only of "ignored" but also of "accepted" things, because the parsing until now only creates a context so it could not really know if something is used, ignored or unknown. Using that list (NastranParser::ACCEPTED_CASE_CONTROL_COMMANDS) it now seems to behave as expected . We might have some commands that should be added to the list, of course.

ThomasAbballe commented 4 years ago

3b21246 solves the issues, in my opinion. We will fill the needed lists as progresses are made.