Key Error when running `AC-MADS-TEM.jl` with target data containing compartments (E.g. VEGC, VEGN) #738

Benjamin-Maglio · 2024-09-06T14:32:14Z

The following error is occurring when loading target data in AC-MADS-TEM.jl for variables with compartments such as vegetation carbon or nitrogen:

ERROR: LoadError: PyError ($(Expr(:escape, :(ccall(#= /home/develop/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:43 =# @pysym(:PyObject_Call), PyPtr, (PyPtr, PyPtr, PyPtr), o, pyargsptr, kw))))) <class 'KeyError'>
KeyError(0)
  File "/work/scripts/drivers/MadsTEMDriver.py", line 312, in observed_vec
    targets = [self.targets[ct][PFT] for PFT in range(10) if PFT in pftnums]
  File "/work/scripts/drivers/MadsTEMDriver.py", line 312, in <listcomp>
    targets = [self.targets[ct][PFT] for PFT in range(10) if PFT in pftnums]

Stacktrace:
  [1] pyerr_check
    @ ~/.julia/packages/PyCall/1gn3u/src/exception.jl:75 [inlined]
  [2] pyerr_check
    @ ~/.julia/packages/PyCall/1gn3u/src/exception.jl:79 [inlined]
  [3] _handle_error(msg::String)
    @ PyCall ~/.julia/packages/PyCall/1gn3u/src/exception.jl:96
  [4] macro expansion
    @ ~/.julia/packages/PyCall/1gn3u/src/exception.jl:110 [inlined]
  [5] #107
    @ ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:43 [inlined]
  [6] disable_sigint
    @ ./c.jl:458 [inlined]
  [7] __pycall!
    @ ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:42 [inlined]
  [8] _pycall!(ret::PyCall.PyObject, o::PyCall.PyObject, args::Tuple{}, nargs::Int64, kw::Ptr{Nothing})
    @ PyCall ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:29
  [9] _pycall!
    @ ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:11 [inlined]
 [10] #_#114
    @ ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:86 [inlined]
 [11] (::PyCall.PyObject)()
    @ PyCall ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:86
 [12] top-level scope
    @ /work/mads_calibration/AC-MADS-TEM.jl:111
in expression starting at /work/mads_calibration/AC-MADS-TEM.jl:111

The text was updated successfully, but these errors were encountered:

tobeycarman · 2024-09-06T15:06:48Z

@Benjamin-Maglio just noticed that there is a comment about compartments right before line 312 in MadsTEMDriver.py:
Maybe try modifying as suggested in the comment and see what happens? Notice the typo in self in the comment.
Probably will need an else clause too. If the type is a single value, then its a soil variable, if its a list then its a PFT variable, if its a dict, then its a PFT/Compartment variable. Maybe this is not the absolute best solution, but it might work?

valeriabriones · 2024-09-09T18:16:52Z

@tobeycarman when I tried that I get this similar but different error:

ERROR: LoadError: PyError ($(Expr(:escape, :(ccall(#= /home/develop/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:43 =# @pysym(:PyObject_Call), PyPtr, (PyPtr, PyPtr, PyPtr), o, pyargsptr, kw))))) <class 'TypeError'>
TypeError("'int' object is not subscriptable")
File "/work/scripts/drivers/MadsTEMDriver.py", line 312, in observed_vec
targets = [self.targets[ct][PFT] for PFT in range(10) if PFT in pftnums]
File "/work/scripts/drivers/MadsTEMDriver.py", line 312, in
targets = [self.targets[ct][PFT] for PFT in range(10) if PFT in pftnums]

tobeycarman · 2024-09-09T19:19:04Z

@valeriabriones @Benjamin-Maglio, wow, on account of your issues, I just found a separate, but kinda related bug!! In MadsTEMDriver.py: there is an indentation error with the return statement in MadsTEMDriver.observed_vec() that was allowing the test to pass despite totally flawed logic!

Notice that the test actually includes a target with compartment values (VegCarbon), but the test was passing. Basically because the return statement is indented one level too far.

Here is one question that comes up: what do you want the returned vector to look like? Still a flat list enumerating the targets by PFT and compartment, i.e. like this:

[vegc_pft0_leaf, vegc_pft0_stem, vegc_pft0_root, vegc_pft1_leaf, vegc_pft1_stem, ...]

or organized some other way?

valeriabriones · 2024-09-09T20:57:40Z

@tobeycarman I think a flat list still works well for the current workflow we have set up! So the indentation allowed it to pass although it wasn't working properly?

tobeycarman · 2024-09-09T21:39:36Z

@valeriabriones ok, so that order is good? Or would you rather have it in a similar order to how they are listed in the targets?
Option (1) looks like this:

[
  GppAllignN pft1, pft2, ... pft6
  vegC pft1 leaf, vegc pft1 stem, vegC pft1 root, 
  vegC pft2 leaf, vegc pft2 stem, vegC pft2 root, 
 ...
  vegC pft6 leaf, vegc pft6 stem, vegC pft6 root, 

]

Option (2) looks like this (more like the existing calibration_targets.py data structure):

[
  GppAllignN pft1, pft2, ... pft6
  vegC pft1 leaf, vegc pft2 leaf, vegC pft3 leaf,  ... vegC pft6 leaf,
  vegC pft1 stem, vegc pft2 stem, vegC pft3 stem, ... vegC pft6 stem,
  vegC pft1 root, vegc pft2 root, vegC pft3 root, ... vegC pft6 root,
  ...
]

In either case, it seems like the totally flattened list might be kind of error prone to work with as the number of targets increases??? It would be up to the user of the MadsTEMDriver.observed_vec() to make sure that they are correctly indexing into the flat list...not mixing up the order of PFTs or the compartments...
Maybe it would be better to return nested lists or some other data structure...?

I think that so far this has not been a problem because y'all haven't been using this MadsTEMDriver.observed_vec() function in any post processing and have been reading the target data a different way? I think that the MadsTEMDriver.observed_vec() might have been something I added so that I could do the testing - looks from the comment that is might exist there as a way to print out the target data during setup and before actually running the model.

valeriabriones · 2024-09-09T21:43:28Z

@tcarman you're right I think we're only referencing this function within the MADS calibration only and not in other calibration functions, so maybe it wouldn't disrupt anything to modify the output format. Maybe Format Option 1 would make sense, but would be good to get your thoughts @Benjamin-Maglio @jsclein-uaf ?

tobeycarman · 2024-09-09T22:19:31Z

Yeah, maybe a better option is to return a richer data structure, like a pandas DataFrame, something like this:

d = drivers.MadsTEMDriver.MadsTEMDriver(config_dict)

# ... load targets, etc, etc

import pandas as pd
pd.DataFrame( d.observed_vec() ) # <-- maybe would want to rename func since its no longer returning a plain vector
        cmtnum                  ctname  pft  observed cmprt
    0        6  GPPAllIgnoringNitrogen    0    11.833   NaN
    1        6  GPPAllIgnoringNitrogen    1   197.867   NaN
    2        6  GPPAllIgnoringNitrogen    2    42.987   NaN
    3        6  GPPAllIgnoringNitrogen    3    10.667   NaN
    4        6  GPPAllIgnoringNitrogen    4     3.375   NaN
    5        6  GPPAllIgnoringNitrogen    5    16.000   NaN
    6        6  GPPAllIgnoringNitrogen    6     6.000   NaN
    7        6               VegCarbon    0     2.000  Leaf
    8        6               VegCarbon    0     4.000  Stem
    9        6               VegCarbon    0     0.297  Root
    10       6               VegCarbon    1    37.100  Leaf
    11       6               VegCarbon    1     0.000  Stem
    12       6               VegCarbon    1   161.280  Root
    13       6               VegCarbon    2     8.060  Leaf
    14       6               VegCarbon    2     0.000  Stem
    15       6               VegCarbon    2    11.040  Root
    16       6               VegCarbon    3     2.000  Leaf
    17       6               VegCarbon    3     0.000  Stem
    18       6               VegCarbon    3     3.200  Root
    19       6               VegCarbon    4     2.000  Leaf
    20       6               VegCarbon    4     0.000  Stem
    21       6               VegCarbon    4     0.000  Root
    22       6               VegCarbon    5    22.000  Leaf
    23       6               VegCarbon    5     0.000  Stem
    24       6               VegCarbon    5     0.000  Root
    25       6               VegCarbon    6    23.000  Leaf
    26       6               VegCarbon    6     0.000  Stem
    27       6               VegCarbon    6     0.000  Root

Benjamin-Maglio · 2024-09-10T13:18:58Z

@valeriabriones I agree with option 1, I think that also matches sensitivity too?

@tobeycarman, how do you foresee this structure being used? Currently, I'm only really concerned with having MADs run, but is there a benefit to using this for something else?

tobeycarman · 2024-09-10T19:12:30Z

@Benjamin-Maglio good point - I don't have a direct use, maybe just thinking if I were to consume the data it would be less error prone to have it be labeled. In any case I ended up with both implementations (flat and labeled) as it ended up being easier way to try testing the expected order of the list. I will admit I am not sure I understand the implications of the previous bug with the return value of the MadsTEMDriver.observed_vec(...) function.

Benjamin-Maglio · 2024-09-10T19:20:06Z

@tobeycarman Yeah I mean the labeled data is much nicer, and I could see there being some application at some point...
So that PR should fix our original bug? Have you tried a mads run with VEGC? Or I could try that?

tobeycarman · 2024-09-10T20:13:08Z

@Benjamin-Maglio I did not try full MADS run. That doctest script uses the MadsTEMDriver.run() function, but that doesn't really invoke anything with MADS. If you are setup to try it that would be great. I expect that PR to fix the original bug but maybe uncover more :-),

valeriabriones · 2024-09-11T16:43:59Z

@tcarman @Benjamin-Maglio I implemented the changes and am still getting some syntax errors starting at the line in AC-MADS-TEM.jl targets = dvmdostem.observed_vec(format= 'format')

ERROR: LoadError: syntax: character literal contains multiple characters
Stacktrace:
[1] top-level scope
@ /work/mads_calibration/AC-MADS-TEM.jl:111
in expression starting at /work/mads_calibration/AC-MADS-TEM.jl:111

tobeycarman · 2024-09-11T17:09:37Z

Oh! My bad - Julia likes double quotes for strings, single quotes for single chars. So if you change to double quotes, it should help

…

On Wed, Sep 11, 2024 at 8:44 AM valeriabriones ***@***.***> wrote: @tcarman <https://github.com/tcarman> @Benjamin-Maglio <https://github.com/Benjamin-Maglio> I implemented the changes and am still getting some syntax errors starting at the line in AC-MADS-TEM.jl targets = dvmdostem.observed_vec(format= 'format') ERROR: LoadError: syntax: character literal contains multiple characters Stacktrace: [1] top-level scope @ /work/mads_calibration/AC-MADS-TEM.jl:111 in expression starting at /work/mads_calibration/AC-MADS-TEM.jl:111 — Reply to this email directly, view it on GitHub <#738 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGMYT6FI72VUUSHXFEUUZLZWBXOLAVCNFSM6AAAAABNYWW4VGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBUGE2TKNRQGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

valeriabriones · 2024-09-11T17:31:28Z

@tobeycarman thank you that did the trick! Also the return targets should be aligned with the last if statement, or else it returns a call error. Not sure if I can edit the commit once its 'pending'?

tobeycarman · 2024-09-12T14:36:30Z

@valeriabriones hey so were you able to test this by checking out the branch #iss738-mads-targets-compartments or did you kind of manually copy the code into the branch you are working on? In that branch I think I have the return statement set at the correct indentation level because the doctest tests pass - but I obviously didn't check the AC-MADS-TEM.jl script...
Let me know if you want to meet quickly to discuss...

valeriabriones · 2024-09-12T16:04:47Z

@tobeycarman No I did a hack and manually copied the revised code into my current set up on master

valeriabriones · 2024-09-20T13:30:57Z

@tobeycarman tried testing the revised MADS on a calibration run with soil and vegetation but ended up getting this new error:

haven't seen this error before. Any help appreciated

valeriabriones · 2024-09-24T03:44:44Z

@tcarman @Benjamin-Maglio I was able to complete all MADS calibration runs (soil, veg, veg + soil), however it seems with the updated code there are still some issues. Only the MADS with just vegetation as the target produced the iteration files. Running separate valibrations for soil and veg+soil did not produce the final files. The error produced when running separate MADS for soil, soil+veg were both similar:

Seems to be 'looking' for a pft when there is none for soil targets
While vegetation did complete and produce the final files, no plot was saved and produced this plotting error, which I can try to look at a bit further:

tobeycarman · 2024-09-24T05:56:36Z

OK. Thats good - it seems to be getting past the "expensive" part (the Mads.calibrate(md, tolOF=0.01, tolOFcount=4), approx line 238, and is failing when trying to build nicely named lists for the plotting that happens later. In both cases, the .iterationresults and .finalresults files should exist, but may not have been copied into their final location yet. In the second screen capture, it looks like the out_labels are not quite right - seems like they should have the pft compartment in there too ?

For the record, we met offline and updated @valeriabriones workstation and containers to v0.8.0 and then pulled #741 into a temporary integration branch for testing.

Benjamin-Maglio · 2024-09-24T15:46:31Z

I am still hitting a similar error to earlier:

ERROR: LoadError: PyError ($(Expr(:escape, :(ccall(#= /home/develop/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:43 =# @pysym(:PyObject_Call), PyPtr, (PyPtr, PyPtr, PyPtr), o, pyargsptr, kw))))) <class 'TypeError'>
TypeError("'float' object is not subscriptable")
  File "/work/scripts/drivers/MadsTEMDriver.py", line 354, in observed_vec
    targets.append(self.targets[ct][PFT])

Stacktrace:
  [1] pyerr_check
    @ ~/.julia/packages/PyCall/1gn3u/src/exception.jl:75 [inlined]
  [2] pyerr_check
    @ ~/.julia/packages/PyCall/1gn3u/src/exception.jl:79 [inlined]
  [3] _handle_error(msg::String)
    @ PyCall ~/.julia/packages/PyCall/1gn3u/src/exception.jl:96
  [4] macro expansion
    @ ~/.julia/packages/PyCall/1gn3u/src/exception.jl:110 [inlined]
  [5] #107
    @ ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:43 [inlined]
  [6] disable_sigint
    @ ./c.jl:458 [inlined]
  [7] __pycall!
    @ ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:42 [inlined]
  [8] _pycall!(ret::PyCall.PyObject, o::PyCall.PyObject, args::Tuple{}, nargs::Int64, kw::PyCall.PyObject)
    @ PyCall ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:29
  [9] _pycall!
    @ ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:11 [inlined]
 [10] (::PyCall.PyObject)(; kwargs::Base.Pairs{Symbol, String, Tuple{Symbol}, NamedTuple{(:format,), Tuple{String}}})
    @ PyCall ~/.julia/packages/PyCall/1gn3u/src/pyfncall.jl:86
 [11] top-level scope
    @ /work/mads_calibration/AC-MADS-TEM.jl:111
in expression starting at /work/mads_calibration/AC-MADS-TEM.jl:111

I'll try and follow the approach above (updated workstation, containers, and create and temporary integration branch)

Benjamin-Maglio · 2024-09-25T13:29:17Z

Ok, so I followed a similar approach above updating containers and creating a temporary integration branch and this error has gone. I am now getting the same error as @valeriabriones.

I'm going to try and fix this issue today by working through the plotting in Python / Julia terminal

valeriabriones · 2024-10-23T18:10:56Z

@tobeycarman @Benjamin-Maglio Not sure if you have come across any issues since the updates with the AC-MADS-TEM.jl, but after running calibration a few times now it seems there is something going on where post-MADS optimal targets are returning as NaN or zero for some/ all of the targets:

When running with GPPAllIgnoringNitrogen as the target all returned as zeros.
Of course, nothing is failing, just requires a workaround where I run a single run with the opt_params to visualize what the targets are

Benjamin-Maglio · 2024-10-23T18:40:11Z

Weird, I haven't encountered this. So are the values NaNs or zeros in the .finalresults file?

valeriabriones · 2024-10-23T19:26:45Z

Ok, I can dig a big further. The parameter values write out correctly, but the OF and lamda values in the .finalresults are NaN as well

Benjamin-Maglio · 2024-10-24T12:04:39Z

I'll try and take a look at this today. Is this happening for a specific configuration (e.g. compartment, pft specific variables)? Or particular parameters?

valeriabriones · 2024-10-24T16:09:29Z

It seems to be with both soil and non-soil targets similarly with parameters Its happened with kdcs and soil as well as cmax and INGPP.

Benjamin-Maglio · 2024-10-24T16:26:27Z

Ok, I will try to replicate this issue!

valeriabriones mentioned this issue Sep 9, 2024

Issue when running calibration for a target with [leaf,stem,root] #685

Closed

tobeycarman linked a pull request Sep 10, 2024 that will close this issue

Modify/improve MadsTEMDriver.observed_vec function, fix bug. #741

Merged

tobeycarman closed this as completed in #741 Oct 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Key Error when running `AC-MADS-TEM.jl` with target data containing compartments (E.g. VEGC, VEGN) #738

Key Error when running `AC-MADS-TEM.jl` with target data containing compartments (E.g. VEGC, VEGN) #738

Benjamin-Maglio commented Sep 6, 2024

tobeycarman commented Sep 6, 2024

valeriabriones commented Sep 9, 2024

tobeycarman commented Sep 9, 2024

valeriabriones commented Sep 9, 2024

tobeycarman commented Sep 9, 2024

valeriabriones commented Sep 9, 2024

tobeycarman commented Sep 9, 2024

Benjamin-Maglio commented Sep 10, 2024

tobeycarman commented Sep 10, 2024

Benjamin-Maglio commented Sep 10, 2024

tobeycarman commented Sep 10, 2024

valeriabriones commented Sep 11, 2024

tobeycarman commented Sep 11, 2024 via email

valeriabriones commented Sep 11, 2024

tobeycarman commented Sep 12, 2024

valeriabriones commented Sep 12, 2024

valeriabriones commented Sep 20, 2024

valeriabriones commented Sep 24, 2024

tobeycarman commented Sep 24, 2024 •

edited

Loading

Benjamin-Maglio commented Sep 24, 2024

Benjamin-Maglio commented Sep 25, 2024

valeriabriones commented Oct 23, 2024

Benjamin-Maglio commented Oct 23, 2024

valeriabriones commented Oct 23, 2024

Benjamin-Maglio commented Oct 24, 2024

valeriabriones commented Oct 24, 2024

Benjamin-Maglio commented Oct 24, 2024

Key Error when running AC-MADS-TEM.jl with target data containing compartments (E.g. VEGC, VEGN) #738

Key Error when running AC-MADS-TEM.jl with target data containing compartments (E.g. VEGC, VEGN) #738

Comments

Benjamin-Maglio commented Sep 6, 2024

tobeycarman commented Sep 6, 2024

valeriabriones commented Sep 9, 2024

tobeycarman commented Sep 9, 2024

valeriabriones commented Sep 9, 2024

tobeycarman commented Sep 9, 2024

valeriabriones commented Sep 9, 2024

tobeycarman commented Sep 9, 2024

Benjamin-Maglio commented Sep 10, 2024

tobeycarman commented Sep 10, 2024

Benjamin-Maglio commented Sep 10, 2024

tobeycarman commented Sep 10, 2024

valeriabriones commented Sep 11, 2024

tobeycarman commented Sep 11, 2024 via email

valeriabriones commented Sep 11, 2024

tobeycarman commented Sep 12, 2024

valeriabriones commented Sep 12, 2024

valeriabriones commented Sep 20, 2024

valeriabriones commented Sep 24, 2024

tobeycarman commented Sep 24, 2024 • edited Loading

Benjamin-Maglio commented Sep 24, 2024

Benjamin-Maglio commented Sep 25, 2024

valeriabriones commented Oct 23, 2024

Benjamin-Maglio commented Oct 23, 2024

valeriabriones commented Oct 23, 2024

Benjamin-Maglio commented Oct 24, 2024

valeriabriones commented Oct 24, 2024

Benjamin-Maglio commented Oct 24, 2024

Key Error when running `AC-MADS-TEM.jl` with target data containing compartments (E.g. VEGC, VEGN) #738

Key Error when running `AC-MADS-TEM.jl` with target data containing compartments (E.g. VEGC, VEGN) #738

tobeycarman commented Sep 24, 2024 •

edited

Loading