-
Notifications
You must be signed in to change notification settings - Fork 1
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
Update 1 #15
Conversation
…del, 2. region dictionary, 3. run metadata). Modified write_ciam() (ciamhelper.jl) and main.jl to reflect this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some random changes here in there. The comment re file reading paths applies to all the file reading instances.
if f==nothing | ||
f=joinpath(dirname(pathof(MimiCIAM)),"../data/batch/init.txt") | ||
end | ||
varnames=CSV.read(open(f),header=true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use CSVFiles.jl instead of CSV to read files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an with CSVFiles handling larger files (>2MB), so sticking with CSV for now
So I think we are still trying to get to a |
Yes, I'm working on that now. |
Updated get_model() to return only the model instance and modified write_ciam() to not require additional inputs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this looks good to me!
Maybe @FrankErrickson or @lrennels could also take a look?
I'll take a look this afternoon! |
src/ciam.jl
Outdated
|
||
return m | ||
return (m) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can just use return m
not return (m)
since the latter is a single tuple which simplifies to the former anyways and thus both return a Model
type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, as in python, (1)
is just an Int, 1. To make it a one-element tuple, you have to add the weird trailing comma, (1,)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry yes that’s what I meant, so to simplify this code there’s really no reason for the parentheses unless the intention was a tuple right?
…ase, as in Diaz (2016). This change affects only NoAdaptation Costs
…y-level detail. To do: add subcosts
…ite optimal fixed costs and subcosts to csv.
Modified get_model() function to require no initial arguments. Output of get_model() is now a tuple of (1: model, 2: segment-region dictionary, and 3: initialization parameters). Using tuple structure because 2 and 3 are needed for the write_ciam() function to write the model output into csv format.