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

Add helper functions to validate and get an epJSON object and field value #7632

Closed
3 tasks
mjwitte opened this issue Nov 22, 2019 · 5 comments
Closed
3 tasks

Comments

@mjwitte
Copy link
Contributor

mjwitte commented Nov 22, 2019

Issue overview

A couple of common patterns for input processing functions that are accessing epJSON data directly are:

From UnitarySystem::getUnitarySystemInputData
                auto const &fields = instance.value();

                if (fields.find("heating_coil_name") != fields.end()) { // not required field
                    loc_m_HeatingCoilName = UtilityRoutines::MakeUPPERCase(fields.at("heating_coil_name"));
                }

From WaterToWaterHeatPumpEIR::processInputForEIRWWHP:
                        if (fields.find("reference_coefficient_of_performance") != fields.end()) {
                            thisWWHP.referenceCOP = fields.at("reference_coefficient_of_performance");
                        } else {
                            Real64 defaultVal = 0.0;
                            if (!inputProcessor->getDefaultValue(cCurrentModuleObject,
                                                                 "reference_coefficient_of_performance", defaultVal)) {
                                // this error condition would mean that someone broke the input dictionary, not their
                                // input file.  I can't really unit test it so I'll leave it here as a severe error
                                // but excluding it from coverage
                                ShowSevereError(  // LCOV_EXCL_LINE
                                        "EIR WWHP: Reference COP not entered and could not get default value"); // LCOV_EXCL_LINE
                                errorsFound = true;  // LCOV_EXCL_LINE
                            } else {
                                thisWWHP.referenceCOP = defaultVal;
                            }
                        }

That's a lot of work to repeat just to get an input field value or it's default.

Also, as evidenced by #7624 a simple typo in the field name reference can result in an input field being missed, because there's no validation that the requested field is a valid field name for the object.

So, it would be useful to have an input processor function that

  1. Takes a field name, validates the field name, and returns the input value or it's default.
  2. To validate field name, will likely have to pass in cCurrentObjectItem (object class name) as an argument.
  3. If the field doesn't have a declared default, then numeric should return zero, and string should return blank.
  4. The EIRWWHP test for no default and no input is unusual, but for objects that want to test this for a given field, perhaps this function could return a status flag? (0 = user input, 1 = default, -1 = no user input or default) or ???

For a field where you simply want to get the value or the default and move on, the goal would be something like:

loc_m_HeatingCoilName = inputProcessor->getFieldValue("heating_coil_name", fields, cCurrentObjectItem, status);

Details

Some additional details for this issue (if relevant):

  • Platform (Operating system, version)
  • Version of EnergyPlus (if using an intermediate build, include SHA)
  • Unmethours link or helpdesk ticket number

Checklist

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • Defect file added (list location of defect file here)
  • Ticket added to Pivotal for defect (development team task)
  • Pull request created (the pull request will have additional tasks related to reviewing changes that fix this defect)
@rraustad
Copy link
Contributor

And for autosized fields returning -99999 would also be helpful in reducing the GetInput code.

if (fields.find("cooling_supply_air_flow_rate") != fields.end()) { // not required field, autosizable
    auto tempFieldVal = fields.at("cooling_supply_air_flow_rate");
    if (tempFieldVal == "Autosize") {
        loc_m_CoolingSAFMethod_SAFlow = DataSizing::AutoSize;
    } else {
        loc_m_CoolingSAFMethod_SAFlow = fields.at("cooling_supply_air_flow_rate");
    }
}

@rraustad
Copy link
Contributor

rraustad commented Jun 3, 2020

Anyone up for tackling this one?

@Myoldmopar
Copy link
Member

This would be really great to get fixed. I'd say @mbadams5 is the most suitable person for handling all the corner cases, but I'm not sure about availability. I can try to hack something up if no one else can.

@mjwitte mjwitte changed the title Add a function to get an epJSON field value with a single line Add helper functions to validate and get an epJSON object and field value Sep 3, 2020
@mjwitte
Copy link
Contributor Author

mjwitte commented Sep 3, 2020

As highlighted here in 8250, it seems that a function to validate that an object type exists within the epJSON schema would be useful and should be a standard piece of every getinput function.

@mjwitte
Copy link
Contributor Author

mjwitte commented Jul 9, 2021

Closed via #8738

@mjwitte mjwitte closed this as completed Jul 9, 2021
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

3 participants