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 unit handling and connection checking #29

Merged
merged 2 commits into from
May 2, 2016

Conversation

jrising
Copy link
Contributor

@jrising jrising commented Apr 26, 2016

This is the component-to-component part of the unit checking system. You add new units like this:

waterdemand = Parameter(index=[regions, time], units="1000 m^3")

and when you call connectparameter with two components, it checks that the units match, allowing a special any unit that matches anything, but otherwise requiring that the units are the same or both missing. I have all my calls like this through lines like:

allocation[:waterdemand] = waterdemand[:totaldemand]

This does not (1) handle any unit checking anywhere except for through connectparameter (and not the version of connectparameter that takes an already-added parameter as an argument, (2) have any logic for the UnitedObject that I mentioned before, nor (3) apply the unit logic to equations.

@jrising
Copy link
Contributor Author

jrising commented Apr 26, 2016

What do you make of the error that this is getting on Linux?

$ if [[ -a .git/shallow ]]; then git fetch --unshallow; fi
fatal: git fetch-pack: expected shallow list

@davidanthoff
Copy link
Collaborator

Given that both appveyor and travis have an error in the git clone step, maybe github was down for a short time and they couldn't fetch? Certainly looks like some sort of timeout problem that is not related to your code at all.

I'll look at the PR once I'm done with my lecture!


# Check the units, if provided
@assert unitcheck(getmetainfo(m, target_component).parameters[target_name].unit,
getmetainfo(m, source_component).variables[source_name].unit) "Units of $source_component.$source_name do not match $target_component.$target_name."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not use @assert here, there is some discussion that those statements might turn into no-ops for higher optimization levels in julia (JuliaLang/julia#10614). Probably safest to just use an if and then throw construction here. I would just throw an ErrorException if things don't match.

@davidanthoff
Copy link
Collaborator

Alright, I've added some smallish comments. Very nice, thanks a lot!

@jrising
Copy link
Contributor Author

jrising commented Apr 30, 2016

Like the commit message says, I tried to address all of these pieces in light of our conversation. In particular:

  • connectparameter can take an ignoreunits=true optional argument to ignore the units.
  • Otherwise, any unit (including "") that does not match will be an error.
  • The term 'units' has been changed to 'unit'.

Plus the other things you asked for:

  • direct throwing rather than asserting.
  • Using AbstractString rather than a Union (I forgot that existed!)
  • Switching the type for unit back to UTF8String... for now.

@davidanthoff davidanthoff merged commit 6dbf789 into mimiframework:master May 2, 2016
@davidanthoff
Copy link
Collaborator

Perfect, thanks a lot!

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

Successfully merging this pull request may close these issues.

2 participants