Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to add new variables to create an input file from scratch #28

Open
aarojas20 opened this issue Jun 15, 2021 · 6 comments
Open

Unable to add new variables to create an input file from scratch #28

aarojas20 opened this issue Jun 15, 2021 · 6 comments

Comments

@aarojas20
Copy link
Contributor

Describe the bug
Hello, I would like to create a .SOL input file from scratch, ie from other experimental data or databases. From what I can understand, I can only edit values from an existing .SOL file. Is there a way to write a .SOL file from scratch, or perhaps I am missing how?

To Reproduce

  1. My main attempt at this was to create a SoilFile object from an empty file with an asterisk in it. (Note, github does not allow me to upload the .SOL file here; hence, I am uploading a .txt file, empty.txt. One can change this back to empty.SOL)
  2. Then, I tried to add sections and setting values from there. For example,
    soil._values.add_section("field_id")
    soil._values.set_value("SALB", 0.2, sect="field_id")
  3. However, because the variable is not in self._values, it cannot add the variable:
    ValueSection.set_value in tradssat/tmpl/vals.py
    258 if not success:
    --> 259 raise ValueError('Variable "{}" not found.'.format(var))

empty.txt

Expected behavior
Given the known allowable variables, ie SoilFile.variables(), it would be great to add values to an input soil file. Please let me know if this enabled or if you can you provide guidelines on how to include this. Happy to work on a subsequent PR if that helps.

@julienmalard
Copy link
Owner

Hello,

Thanks for this suggestion! This would indeed be an important functionality. If you would like to work on a PR, that would be very much appreciated...at the same time I have been wondering whether this might be a good time to re-work traDSSAT a bit to use pandas as internal storage for data. If you think that using pandas internally could make this pull request easier, perhaps we could collaborate on getting this (probably inevitable, in the long run!) change done?
Looking forward to your thoughts!

@aarojas20
Copy link
Contributor Author

aarojas20 commented Jun 22, 2021

Thanks @julienmalard ,
I did a little more diving to understand how I can add variables and their associated data after adding a section called "field_id".

Here is an excerpt,

lvars = [
    CharacterVar('SITE', 11, info='Site name'),
    CharacterVar('COUNTRY', 12, info='Country name'),
    FloatVar('LAT', 8, 3, info='Latitude'),
    FloatVar('LONG', 7, 3, info='Longitude'),
    CharacterVar('SCS FAMILY', 50, info='Family, SCS system')
]

lvals = [
    np.array("field_id", ndmin=1),
    np.array("US", ndmin=1),
    np.array(lat, ndmin=1),
    np.array(lon, ndmin=1),
    np.array("Generic", ndmin=1)
]

sfile._values._sections["field_id"].add_subsection(vals.ValueSubSection(lvars, lvals))

where lat and lon are vars for float numbers.
I can see that adding the section was successful with sfile.to_dict():

{'': {'header vars': {}, 'main vars': []},
 'field_id': {'header vars': {},
  'main vars': [{'SITE': array(['field_id'], dtype='<U8'),
    'COUNTRY': array(['US'], dtype='<U2'),
    'LAT': array([34.3]),
    'LONG': array([-91.6]),
    'SCS FAMILY': array(['Generic'], dtype='<U7')}]}}

I am not sure to whether to be concerned about the first empty section, as a result of instantiating from an empty .SOL file.
I was able to add the remaining sub-sections and write the .SOL file. I have yet to test it in DSSAT to see if the empty section creates an issue. (I am a beginner in DSSAT.)

Moving forward with a PR:

  1. I can base the PR off of something similar as above. The main difference would be to be able to instantiate a SoilFile object without an existing .SOL file. This may also eliminate the empty section as I showed above.
  2. Regarding pandas, I think it would certainly be more intuitive to structure data in that way. Each subsection could be taken as a pd.DataFrame. I am thinking we would still need to call each column from the dataframe to ensure it writes to the .SOL file given the strict row location and column spacing. I am curious about your thoughts on this latter part.

@julienmalard
Copy link
Owner

Hello,

Looks good to me! Which empty section in particular are you referring to - do you mean the '': {'header vars': {}, 'main vars': []}? If it's a problem you could probably just remove it with sfile._values._sections.pop(""). It's a bit of a hack but I'd expect it to work.

It would of course be much better to have a standard (non-hack) way of doing this with a special function. If you can make a PR along those lines (ideally with a test case in the test suite - please let me know if you would need pointers on that) I would be really grateful. Regarding pandas, I think that having each subsection as a dataframe would make the most sense. We would still have to keep the initial variable definitions, etc. in order to parse and write correctly to each DSSAT file (so the dataframe would be for internal purposes only, but would probably make maintaining and improving the code much easier).

What do you think?
Thanks again!

@aarojas20
Copy link
Contributor Author

Hi @julienmalard ,

Thanks for the solution to removing the empty section!

Thanks for the feedback as well. That sounds good; I'll work on a PR to integrate the above. If I have any follow-up questions I'll let you know.

@julienmalard
Copy link
Owner

Thanks @aarojas20 ; I'll look forward to your PR then.

@aarojas20
Copy link
Contributor Author

Hi @julienmalard, I've been working on this in between other projects, and it has taken me longer than expected as a result. I was able to create a new .SOL, .WTH, and a new fileX from scratch; however, the FileX was not recognized by DSSAT. I tried a more simple exercise with an existing fileX provided by DSSAT, and I found that the newly written fileX was not recognized by DSSAT. I have outlined this new issue: #31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants