Make options in some tools more similar to each other for modify_fsurdat and subset_data

[ekluzek]

There are some tools that use different terms for doing essentially the same thing. We should consolidate these to one way of doing the same thing.

modify_fsurdat and subset_data all have some similar options:

The following list of options operate on the same things, but call them something different
modify_fsurdat: configure file options
dom_plant
zero_nonveg
std_elev
max_sat_area
subset_data:
--dom_pft
--include-nonveg
--uniform-snowpack (sets STD_ELEV to 20)
--cap-saturation (sets FMAX to zero)

@negin513 pointed this out in #1615

[ekluzek]

Also modify_fsurdat should have a line in the tools README file, and likely a README file in it's directory as well. And we should have some documentation that describes when you use it as opposed to subset_data.

[slevis-lmwg]

Thank you, @ekluzek , this makes sense to me.

I think it also makes sense to make each tool even more specific in its purpose. In this case subset_data would specialize in subsetting global fsurdat files to regional or 1x1, while fsurdat_modifier would specialize in modifying global fsurdat files. This way users would understand which tool to use for what purpose and would proceed in this order according to need:

• mksurfdata to generate new global fsurdat
• fsurdat_modifier to modify global fsurdat
• subset_data to go from global to regional or 1x1

[ekluzek]

We've scheduled a meeting in a few weeks to discuss this with a subgroup.

Note, that currently subset_data when used for a single-point also modifies the dataset with some options. I think this is still good to do, as otherwise we'd need to run subset_data and then a modify script afterwards. This makes it a one step process for most single-point datasets.

Anyway, we'll have the subgroup discuss and present recommendations about moving forward.

[wwieder]

Hi Erik. if you think it's helpful I'm happy to come to these meetings.

…

On Thu, Mar 3, 2022 at 1:55 PM Erik Kluzek ***@***.***> wrote: We've scheduled a meeting in a few weeks to discuss this with a subgroup. Note, that currently subset_data when used for a single-point also modifies the dataset with some options. I think this is still good to do, as otherwise we'd need to run subset_data and then a modify script afterwards. This makes it a one step process for most single-point datasets. Anyway, we'll have the subgroup discuss and present recommendations about moving forward. — Reply to this email directly, view it on GitHub <#1662 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5IWJGD6RNIVQAFDYO44A3U6ERNFANCNFSM5PRXQ2XA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[wwieder]

In our call today we agreed that this fell into the category of wouldn't it be nice feature, but not an issue that's critical to address right now.

@negin513 noted that if the modify_fsurdat tool is something that is used beyond its immediate user community we can work to harmonize the user interface and code interoperability.

@slevisconsulting noted that the duplication here reflects the simultaneous work that was being done for two different projects.

@adrifoster emphasized the value of creating a design document for each project / PR to help guide our coding practices.

@ekluzek agreed that this isn't critical to address right now, but something to be aware of moving forward.

All, please add to these notes if I missed anything

[slevis-lmwg]

Let me just add that I will be happy to spend time on eliminating redundancy and making the user interfaces more similar between these tools, but such time would need to be signed off by Isla Simpson and Scott Bachman.

[wwieder]

Thanks @slevisconsulting , do you want to ask Scott and Isla about this? If they're not supportive of you spending time on this, maybe we should close the issue with a won't fix?

[slevis-lmwg]

do you want to ask Scott and Isla about this?

I will raise the question at an upcoming meeting with them.

[slevis-lmwg]

Good news:
Isla and Scott support my charging CSSI time toward eliminating redundancy and making the user interfaces more similar between these tools.

[wwieder]

this is good news! thanks for following up, Sam.

…

On Sat, Apr 9, 2022 at 4:39 PM Samuel Levis ***@***.***> wrote: Good news: Isla and Scott support my charging CSSI time toward eliminating redundancy and making the user interfaces more similar between these tools. — Reply to this email directly, view it on GitHub <#1662 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5IWJAGJSLVRCQP2XXW4BDVEIBI5ANCNFSM5PRXQ2XA> . You are receiving this because you commented.Message ID: ***@***.***>

[slevis-lmwg]

Adding a relevant thought here to avoid forgetting:
The write_output function has the potential of looking the same in these tools (modify_fsurdat, subset_data, and modify_mesh #1677), so we may be able to avoid repetition there, too.

I moved the write_output function to utils.py in #1677 , so other tools will have access to it when that PR gets merged to main.

[slevis-lmwg]

I wanted to address the remaining similar tool-options that I left unaddressed in the PR (#1714).

@ekluzek wrote:

The following list of options operate on the same things, but call them something different
modify_fsurdat: configure file options
dom_plant
std_elev
max_sat_area
subset_data: command-line options (<-- last 3 words added by @slevisconsulting )
--dom_pft
--uniform-snowpack (sets STD_ELEV to 20)
--cap-saturation (sets FMAX to zero)

I did not address these options in the PR for the following reasons:

• dom_plant represents pft (plant functional type) or cft (crop functional type), so it differs from dom_pft (but let me know if this has changed)
• std_elev allows the user to enter any valid STD_ELEV, while uniform-snowpack sets STD_ELEV to 20
• max_sat_area allows the user to enter any valid FMAX, while cap-saturation sets FMAX to 0

I could see trying to replace subset_data functions such as write_output and update_metadata with similar corresponding functions that reside in utils.py.

[ekluzek]

@slevisconsulting your explanation above makes sense to me.

However, if I understand both scripts I do think that dom_plant and dom_pft represent the same thing. If you want to set something to a crop CFT you set with --dom_pft in subset_data. So I think we should use dom_pft in modify_fsurdat. I think you are right that this was different in earlier versions of subset_data.

[ekluzek]

@slevisconsulting is this taken care of now? Or is there more to do here?

[slevis-lmwg]

@slevisconsulting is this taken care of now? Or is there more to do here?

Yes, I consider this done. At some point -- I don't rememer when -- I changed dom_plant to dom_pft for consistency with subset_data.