-
Notifications
You must be signed in to change notification settings - Fork 35
Add multiplier and allow_missing macro use #816
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #816 +/- ##
==========================================
- Coverage 84.25% 84.12% -0.14%
==========================================
Files 39 40 +1
Lines 3551 3552 +1
==========================================
- Hits 2992 2988 -4
- Misses 559 564 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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 minor comments on the tutorial language
src/components/README.md
Outdated
|
||
* `adder.jl` -- Defines `Mimi.adder`, which simply adds two parameters, `input` and `add` and stores the result in `output`. | ||
|
||
* `multiplier.jl` -- Defines `Mimi.multiplier`, which simply multiplies two parameters, `input` and `add` and stores the result in `output`. |
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.
The second parameter here is 'multiply' right?
docs/src/internals/structure.md
Outdated
## 3. Pre-compilation and built-in components | ||
|
||
To get `__precompile__()` to work required moving the creation of "helper" components to an `__init__()` method in Mimi.jl, which is run automatically after Mimi loads. It defines the two "built-in" components, from `adder.jl` and `connector.jl` in the `components` subdirectory. | ||
To get `__precompile__()` to work required moving the creation of "helper" components to an `__init__()` method in Mimi.jl, which is run automatically after Mimi loads. It defines the two "built-in" components, from `adder.jl`, `multiplier.jl`, and `connector.jl` in the `components` subdirectory. |
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.
Are there three "built-in" components now?
This PR (1) adds a
multiplier
build in component to help with FUND-FAIR unit conversion and (2) per suggestion from #560 adds an@allow_missing
wrapper to theadder
andmultiplier
components, which also simplifies MimiFUND-MimiFAIR-Flat.This means that if the multiplier component:
accesses a
missing
inp.input[t]
it will not throw an error but just pass the value through. Sometimes needing this can be avoid with setting afirst
andlast
for themultiplier
component, but sometimes there's still an irritating off-by-one issue. For example in MimiFUND-MimiFAIR-Fla we havewhere the
:mco2
from FUND's:emission
ismissing
for the first year FUND runs (1950), and thus we would needmultiplier
to run fromFUND_first + 1
toFUND_last
... or do something fancy with backups which also works. In my opinion we want these to be sort of no-brainer pass-through components that don't do anything fancy so I'd rather just do@allow_missing
but we can discuss if you'd like.