nasa / cape

Computational Aerosciences Productivity & Execution
Other
22 stars 9 forks source link

ApplyDict() not working as expected #4

Closed rbreslavsky closed 1 year ago

rbreslavsky commented 1 year ago

When populating a Config dictionary in pyFun.json with a Components list and an Inputs dictionary, the resulting fun3d.*.nml file lists all components in &component_parameters twice. This does not seem to impact the execution of FUN3D, but it can result in a longer than necessary namelist file.

nasa-ddalle commented 1 year ago

The intended usage here is to not use the Inputs dictionary if you have a valid config file. This may be impacting FUN3D more than you expect; we had a similar issue (just had the same entry in Components twice) that caused FUN3D to sometimes write corrupted iterative history files, probably due to a race between two processes trying to write the same file.

nasa-ddalle commented 1 year ago

I don't think I understood the problem correctly; I wasn't able to reproduce it as I interpreted the issue. It sounds like something is happening if you use "Components" > "Config" (along with a "Config" > "File") and also specify entries in "Config" > "Inputs". When I tried that, the "Inputs" setting just overrode the automatic entry, and I didn't see duplicated &component_parameters entries...

rbreslavsky commented 1 year ago

Ah, I found out what was causing this. It was the use of a NamelistFunction file in which I used cntl.Namelist.ApplyDict(). That last method seems to lead directly or indirectly to a duplication of the components in &component_parameters.

rbreslavsky commented 1 year ago

Hey Derek; just wanted to mention that this is still an issue. Was it a deliberate choice for ApplyDict() to append to already existing dictionary values rather than replacing them?

nasa-ddalle commented 1 year ago

No ... It's not intended, which I can tell because the last line of Namelist.SetVar() (which is called by Namelist.ApplyDict()) is a call to ReplaceOrAddLineToSectionSearch(). As the function's name suggests, this should only append if the value is not present. Most likely this function is having a hard time with namelist files' crazy indexing syntax.

I'm not sure which particular weakness you're encountering ... but it should be something like this where function is confused about lines like

component_name(7) = "something"

It's possible this can be fixed enough to be satisfactory, but I don't think the current namelist module can really close this issue. I have a very different nmlfile module in development that won't have these issues (but it won't attempt to preserve the original text of your namelist template file).

nasa-ddalle commented 1 year ago

Despite some known limitations of this module, I wasn't able to reproduce what I thought would be the closest match for your specific issue. Can you share a few lines that get duplicated after ApplyDict()?

rbreslavsky commented 1 year ago

Let's say I have 10 components in &component_parameters. At the end of listing all 10 components in the .nml file, the whole list starts repeating again.

    component_input(1) = "1"
    component_name(1) = "first_component"
    ...
    component_name(10) = "last_component"
    number_of_components = 10
    component_input(1) = "1"
    component_name(1) = "first_component"
    ...
    number_of_components = 10

I can solve this by doing del nml_dict['component_parameters]. Then when I use ApplyDict() I only get the one occurrence.

nasa-ddalle commented 1 year ago

I'm currently unable to get this same problem using the main branch. See for example:

>>> from cape.pyfun import namelist
>>> nml = namelist.Namelist("fun3d.01.nml")
>>> print("".join(nml.Section["component_parameters"][:20]))
 &component_parameters
    allow_flow_through_forces = .true.
    number_of_components = 277
    component_count(1) = -1
    component_name(1) = "ORION"
    component_input(1) = "7-29"
    ...
    component_input(2) = "7-222,428-429,434"
    ...
    component_name(2) = "CORE_no_base"

Then making up some inputs to ApplyDict(), I get the expected results.

>>> comp_name = ["ORION1", "ORION2", "ORION3"]
>>> data = {"component_parameters": {"component_name": comp_name}}
>>> nml.ApplyDict(data)
>>> print("".join(nml.Section["component_parameters"][:20]))
 &component_parameters
    allow_flow_through_forces = .true.
    number_of_components = 277
    component_count(1) = -1
    component_name(1) = "ORION1"
    component_input(1) = "7-29"
    ...
    component_input(2) = "7-222,428-429,434"
    ...
    component_name(2) = "ORION2"

Since you have a solution, and the whole approach to namelists will change in version 1.1, I'm going to close this issue. It's possible that a recent commit involving cape.pyfun.namelist or cape.filecntl.namelist helped, but I'm not sure.

The challenge occurs because Namelist.SetVar(), which is called by ApplyDict(), is looking for lines of text to change the namelist rather than the values it creates. For some arrays, e.g. [0, 0, 1, 1], there are multiple ways to write that as a namelist. This text-based approach has little hope of catching them all. In the new cape.nmlfile approach, difficulties with weird indexing, e.g.

component_count(:) = 10 * -1

or

plane_normal(1:3,3) = 0.0 1.0 0.0

should go away.

In thinking about this, I think the little r added to before a string in line 230 of namelist.py in https://github.com/nasa/cape/commit/24816dfcfa2aaeadc6dc24aa6c18108375750eef is probably why I'm not getting the same result.

nasa-ddalle commented 1 year ago

Thanks to #17 I finally understand what was happening here and why I couldn't reproduce it. Looks like you were using parentheses on the left-hand side (key names) of the Python dict. Pretty reasonable, but I never thought of that.

(I was just using

{
    "component_name" = [
        "component1",
        "component2",
        ...
    ],
}

which was working fine, but couldn't handle 2D arrays like sampling_parameters > plane_normal very well.)

Anyway, I think this duplication should now be fixed as a result of https://github.com/nasa/cape/commit/3abc1cb4962975c4465f6df0a0a2acf99891127e.