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

Trend random values #36

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Trend random values #36

wants to merge 13 commits into from

Conversation

nikdu
Copy link
Collaborator

@nikdu nikdu commented May 3, 2017

Trends will increase/decrease with 0-10% every day. Minor modifications in the application.conf where modifier is set from "0.01" to "0.001" and backintime from "48" to "96".

val nextEntry = products.head
val product = nextEntry._1
val tsNs = nextEntry._2


Copy link
Contributor

Choose a reason for hiding this comment

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

Remove duplicate newline

plan
}

def readTrendsFromFile(startTs: DateTime) : PriorityMap[Product, Long] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function should be removed, it exists in Products.scala


object Trends extends ApplicationConfig with LazyLogging {
private def parseTrendFromFile(filename: String) : Product = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function should be removed, it exists in Products.scala

@@ -23,10 +23,10 @@ gen {
cassandra.element.amount.max=1000

# Scale up or scale down the amount of cdrs generated (1 for realistic values)
modifier=0.01
modifier=0.001
Copy link
Contributor

Choose a reason for hiding this comment

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

Modifier should be 0.01


# Total amount of hours to generate back in time
backintime=48
backintime=96
Copy link
Contributor

Choose a reason for hiding this comment

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

backintime should be 48

these ++ f.listFiles.filter(_.isDirectory).flatMap(recursiveListFiles)
}

val trendsDirPath = Option(System.getProperty("trends.dir")) match {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is using the old version of trendsDirPath, the one in master is now:

val trendsDirPath = Option(System.getProperty("trends.dir")).
getOrElse(getClass.getClassLoader.getResource("trends").getPath)

Copy link
Collaborator

Choose a reason for hiding this comment

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

github did not fix the indentation for that code above.

val files = recursiveListFiles(new File(trendsDirPath))

// Create a priority list out of all products with default timestamp
var pmap = mutable.HashMap.empty[Product, Long]
Copy link
Collaborator

Choose a reason for hiding this comment

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

pmap can be removed as it is not used anymore.

newDay = timeCheck + 1
if (newDay == 8) {newDay = (newDay % 7)}
products = Trends.changeTrends(products)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think these variables are descriptive.
newDay, timeCheck?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also please write some quick comment that describes what this chunk of code does.

newDay = timeCheck + 1
if (newDay == 8) {newDay = (newDay % 7)}
products = Trends.changeTrends(products)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also please write some quick comment that describes what this chunk of code does.


def changeTrends(maptemp: PriorityMap[Product, Long]) : PriorityMap[Product, Long] = {
val e = maptemp.map(p => (p._1.copy(points = p._1.points.map(point => point.copy(
cdrPerSec = (((Random.nextFloat()*0.2) + (-0.1))*point.cdrPerSec) + point.cdrPerSec))), p._2))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please re-structure this huge one-liner?
I would prefer something like

maptemp.map(p =>
(p._1.copy(
    points = p._1.points.map(
        point=> point.copy(

and so on

Copy link
Collaborator

@Lilja Lilja left a comment

Choose a reason for hiding this comment

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

Well done

hourDiffLow/hourDiffHigh
}

def changeTrends(maptemp: PriorityMap[Product, Long]) : PriorityMap[Product, Long] = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please rename to randomizeTrends or something else that is more descriptive

@@ -1,12 +1,18 @@
package se.qvantel.generator

import java.io.File
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused imports, please remove

p => (p._1.copy(
points = p._1.points.map(
point => point.copy(
cdrPerSec = (((Random.nextFloat()*0.2) + (-0.1))*point.cdrPerSec) + point.cdrPerSec))), p._2))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be nice if the 10% value would be configurable in the application.conf file, but not necessary

@codecov
Copy link

codecov bot commented May 10, 2017

Codecov Report

Merging #36 into master will decrease coverage by 1.47%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #36      +/-   ##
==========================================
- Coverage   35.31%   33.84%   -1.48%     
==========================================
  Files          15       15              
  Lines         252      263      +11     
  Branches       12       11       -1     
==========================================
  Hits           89       89              
- Misses        163      174      +11
Impacted Files Coverage Δ
...main/scala/se/qvantel/generator/CDRGenerator.scala 0% <0%> (ø) ⬆️
src/main/scala/se/qvantel/generator/Trends.scala 77.14% <0%> (-15.97%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ab3962...dcf7ac0. Read the comment docs.

@nikdu
Copy link
Collaborator Author

nikdu commented May 10, 2017

Trend points will increase with a value between 0 and 10% every day. A minor bug was detected while testing and it has not been fixed. The graphs tend to "spike" at 12:00 AM when the randomizeTrends (the method which modifies the trend points) is ran. This spike is only attached to the graph which is modified first when randomizeTrends is ran.

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.

5 participants