Conversation
…to goal-seek-update
otfrom
left a comment
otfrom
left a comment
There was a problem hiding this comment.
I think I understand most of what is going on here. I think this probably belongs in another library and there are a few changes I'd like to make to it so we aren't doing things like writing the data out and re-reading it.
I wonder if the first step would be to make it so that run-send-model returns better things that we can report on in better ways using adroddiad, but also allow us to access what is there in memory if we want to. (which would dump almost all of witan.send.output)
| org.hdrhistogram/HdrHistogram {:mvn/version "2.1.9" #_"2.1.12"} | ||
| same/ish {:mvn/version "0.1.4"}} | ||
| same/ish {:mvn/version "0.1.4"} | ||
| witan.send.adroddiad/witan.send.adroddiad {:local/root "../witan.send.adroddiad"}} |
There was a problem hiding this comment.
yeah, I'm worried about this dependency. I think we'll get away with it as we use adroddiad as a library, but you are right that we should split this out of the model.
| (into key-stage-3 key-stage-4)) | ||
|
|
||
| ;; Key Stages 1 to 5 | ||
| (def school-age (reduce #(into %1 %2) (sorted-set) [primary-school secondary-school key-stage-5])) |
There was a problem hiding this comment.
@seb231 I'd like to merge this bit even if we don't merge the rest but move it to another library
src/clj/witan/send/goal_seek.clj
Outdated
| [witan.send.adroddiad.simulated-transition-counts :as stc] | ||
| [witan.send.adroddiad.simulated-transition-counts.io :as stcio] | ||
| [witan.send.adroddiad.summary :as summary])) |
There was a problem hiding this comment.
this would be fine if goal-seek was a separate lib
src/clj/witan/send/goal_seek.clj
Outdated
| (let [result (->> baseline | ||
| (filter #(and (= (:calendar-year %) year) | ||
| (every? identity (witan.send.model.prepare/test-predicates % state-map)))) | ||
| (map #(select-keys % [:population :median])) | ||
| (apply merge-with +))] |
There was a problem hiding this comment.
I think the data is small enough at this point that it doesn't matter that it is ->> rather than a transduce.
src/clj/witan/send/goal_seek.clj
Outdated
| (every? identity (witan.send.model.prepare/test-predicates % state-map)))) | ||
| (map #(select-keys % [:population :median])) | ||
| (apply merge-with +))] | ||
| (get result :population (get result :median)))) |
There was a problem hiding this comment.
Do we need to use summary if we only want the median for goal-seek?
There was a problem hiding this comment.
No probably not. Didn't really think about the fact that we could calculate it separately.
src/clj/witan/send/goal_seek.clj
Outdated
| (filter #(and (= (:calendar-year %) year) | ||
| (every? identity (witan.send.model.prepare/test-predicates % state-map)))) |
There was a problem hiding this comment.
This feels like the important line I need to understand and I don't. I'm not sure why you are selecting things here to be summed later.
src/clj/witan/send/goal_seek.clj
Outdated
| result (tc/dataset (eduction stcio/simulated-transitions->transition-counts-xf (:simulated-transitions projection))) | ||
| census (stc/transition-counts->census-counts result (apply min (:calendar-year result))) | ||
| summary (summary/seven-number-summary census [:calendar-year :setting :need :academic-year] :transition-count) | ||
| target-pop-result (get-baseline-population (tc/rows summary :as-maps) target-year (dissoc m :modify-transition-by))] |
There was a problem hiding this comment.
I think it is possible to write get-baseline-population as a tmd/tc thing that might run faster.
| (let [pred-map (-> pred-map | ||
| (rename-keys {:setting :setting-2, :need :need-2 :academic-year :academic-year-2}) | ||
| (update :setting-2 keyword) | ||
| (update :need-2 keyword)) | ||
| result (filter (fn [t] (every? identity (test-predicates t pred-map))) transitions)] |
There was a problem hiding this comment.
I'm not sure I understand why this is happening.
|
I think we already discussed this, but goal-seek doesn't output any files (or read in again) until it's found the optimum modifier |
|
Agreed that this needs to not be merged and moved to it's only library |
seb231
force-pushed
the
goal-seek-update
branch
from
9b0a3c6 to
d56e1c8
Compare
seb231
force-pushed
the
goal-seek-update
branch
from
d56e1c8 to
f06b7af
Compare
2 participants
No description provided.