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

HT-3072 add narrative and stat tracking to estimate generation #106

Merged
merged 1 commit into from
Jul 30, 2021

Conversation

jsteverman
Copy link
Contributor

In addition to the IC cost, this adds information about the number of unique OCNs and the count/percent of matches. In the old system we also reported the number of usable holdings (MPM, SPM, SER), but that is not readily available to bin/compile_estimated_IC_costs.rb because it is only given a list of OCNs.

@jsteverman jsteverman requested review from mwarin and aelkiss July 27, 2021 15:20
Copy link
Contributor

@mwarin mwarin left a comment

Choose a reason for hiding this comment

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

Looks solid. My only concern is that the temporary org_id is "prospective_member", which I assume means you can't run 2 estimates at the same time for different members, not that we're super likely to ever want to do that.


File.open(ocn_file).each do |line|
ocn = line.to_i
ocns << ocn
waypoint.incr

# Find a cluster
cluster = Cluster.find_by(ocns: ocn.to_i,
Copy link
Contributor

Choose a reason for hiding this comment

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

ocn has already been to_i'd on line 36.

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment pertains to "ocn.to_i" on line 41. I must have clicked wrong.

Copy link
Member

@aelkiss aelkiss left a comment

Choose a reason for hiding this comment

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

Looks good to me as well. prospective_member isn't persistent in mongodb, so we should be able to run multiple estimates simultaneously without issue.

@aelkiss aelkiss merged commit 6cbf482 into master Jul 30, 2021
@aelkiss aelkiss deleted the HT-3072-estimate_generation branch July 30, 2021 12:59
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.

3 participants