Conversation
| if link['source'] in stocks_mapping: | ||
| link_dict = {'_id': link_id} | ||
| link_dict['s'] = stocks_mapping[link['source']] | ||
| link_dict['t'] = idx |
There was a problem hiding this comment.
I originally set link_dict['t'] = link['target'][4:] to set the 't' field of the output acset to the flow id defined by the input amr link's 'target' field.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #41 +/- ##
=======================================
Coverage 87.46% 87.46%
=======================================
Files 6 6
Lines 327 327
Branches 54 54
=======================================
Hits 286 286
Misses 36 36
Partials 5 5 ☔ View full report in Codecov by Sentry. |
a143780 to
baa2b5f
Compare
| params_stock_flow_map[flow_id] = [] | ||
|
|
||
| params_stock_flow_map[flow_id].extend( | ||
| re.findall(r'p\.([^()*+-/ ]+)', flow['ϕf'])) |
There was a problem hiding this comment.
doesn't this assume that a parameter always starts with p.?
There was a problem hiding this comment.
Yes, we assumed that was the case.
| params_stock_flow_map[flow_id].extend( | ||
| re.findall(r'p\.([^()*+-/ ]+)', flow['ϕf'])) | ||
| params_stock_flow_map[flow_id].extend( | ||
| re.findall(r'u\.([^()*+-/ ]+)', flow['ϕf'])) |
There was a problem hiding this comment.
doesn't this assume that a state always starts with u.?
There was a problem hiding this comment.
Yes, we assumed that was the case.
| for parameter in amr['semantics']['ode']['parameters']: | ||
| if parameter['id'].startswith('p_'): | ||
| symbols[parameter['id'][2:]] = sympy.Symbol( | ||
| 'p.' + parameter['id'][2:]) |
There was a problem hiding this comment.
The whole p. and u. thing are implementation details in julia that the state variable of a dynamical system is called u and the parameters are p. Do we want to cannonicalize that in the standard?
There was a problem hiding this comment.
We thought that u. and p. are required/expected in your ACSet implementation. So we parse them when reading in an ACSet and generate them when outputting an ACSet. If that's not the case, we can remove the custom handling of these.
There was a problem hiding this comment.
@jpfairbanks could you confirm if the u. and p. are expected in the ACSet or if we should remove this assumption?
|
@jpfairbanks, with these latest commits, @nanglo123 removed explicit handling of u. and p. so any names will work. Anything else to do on this PR? |
…add acset/amr argument for methods, change t field for links during amr->acset
b01f504 to
c4c8423
Compare
|
We don't have merge permissions here so whoever does, please merge! |
This branch has not been deployed
3 participants
No description provided.