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

WIP discussion of function values to resources #716

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

Conversation

purpleidea
Copy link
Owner

No description provided.

@purpleidea
Copy link
Owner Author

Hey @gelisam I opened this PR (it's based of #704) to discuss passing function values to resources. In playing around first I hit an expected bug in the Into() function. This function pushes a bunch of mcl values INTO a golang struct (the resource). Of course the function part wasn't implemented.

So I started moving things around. And I have the import cycle of Txn <-> Types since I thought FancyFunc should maybe be part of the types package. Although it's a bit different because it has the unusual Txn aspect in there.

Before proceeding with a more complex refactor, I figured I'd check with you if you think this all makes sense or if you had a thought about how instead we'd pass in a simpler FuncValue to resources instead? (I don't expect they'll need the Txn, that's not the kind of functions we'd be passing to the resource I don't think.)

Cheers!

@gelisam
Copy link
Contributor

gelisam commented Sep 23, 2023

Notes from today's meeting:

  • a "timeless" function is a function which takes a single list of input Values to a single output Value.
  • a "timeful" function is the opposite: it plays shenanigans with time, e.g. by returning multiple values over time, or no value at all.
  • in test "name" { func1 => expr }, it's fine if expr itself is not timeless, it can e.g. emit a different FuncValue every second. But every FuncValue it emits must be timeless.
  • if a lambda body contains a timeful expression, or a variable whose value is obtained from a timeful expression, then it is considered timeful, even though it would be possible to compile the lambda so that it emits a different FuncValue every time the value of a captured variable changes. The reason for that is that in CallFunc, the sub-graph is only regenerated when the lambda expression is replaced with something else. When the captured variables inside the lambda change, the corresponding Funcs emit values downstream, into the existing sub-graph. New values flow through the sub-graph and the output of the sub-graph changes, but the sub-graph itself does not have to be replaced.

@gelisam
Copy link
Contributor

gelisam commented Sep 23, 2023

I propose a new static analysis phase after type checking and before Expr.Graph(). The input is a mapping from lambda-bound name to their type, along with an annotation indicating whether each function type inside that type is timeless or timeful. The output is a similarly-annotated type for the Expr. We then check if in test "name" {func1 => expr}, the annotated type is a timeless function, and if it returns a function, that function is itself timeless, etc.

@gelisam
Copy link
Contributor

gelisam commented Sep 30, 2023

Since you opened this PR, I guess this time I'll be the one pushing to a auxiliary branch and you'll be the one updating the official MR branch to incorporate my changes! I have pushed to gelisam/functions-to-resources. So far it's the same as master, because now that #704 has been merged, we should obviously start from the merged version.

@gelisam
Copy link
Contributor

gelisam commented Sep 30, 2023

Could you post the examples and counter-examples of what counts as timeless, both as a reminder of what the goal is and so we can use them as tests?

@purpleidea
Copy link
Owner Author

purpleidea commented Sep 30, 2023

I'll fix and throw away this branch. But if you'd prefer to have the PR branch, lmk.

Could you post the examples and counter-examples of what counts as timeless, both as a reminder of what the goal is and so we can use them as tests?

In short:

timeless:

  • any pure function
  • iter.map
  • strconv.itoa

timeful:

  • world.schedule
  • datetime.now
  • os.system

@purpleidea
Copy link
Owner Author

Here are the notes I took (may contain errors)

import "strconv"

$fn1 = func($x) { # pure! also good
	strconv.itoa($x)
}

$fn2 = if datetime.now() % 2 == 0 { # should be good! in fact, this has to work.

	func($x) { # middle point
		strconv.itoa($x)
	}

} else {

	func($x) {
		"hello"
	}

}

$fn3 = func($x) { # bad
	if datetime.now() % 2 == 0 { # this stream function makes things bad (it's also not pure)
		strconv.itoa($x)
	} else {
		"hello"
	}
}

$fn4 = func($x) {
	iter.map(...) # map is pure and okay here, as long as the input function is also.
}

$fn5 = func($x) { # bad!
	os.system("seq $x")
}


$fn6_prep = func($sam) { # this is ok or is it? -- the simple thing to do is that this is not allowed.
	func($x int) {
		$sam
	}
}

$fn6 = $fn6_prep(os.system("seq 10"))


$fn7_prep = func($sam) { # this is good

	if datetime.now() % 2 == 0 { # should be good! in fact, this has to work.

		func($x) { # middle point
			strconv.itoa($x)
		}

	} else {

		func($x) {
			"hello" + $sam
		}

	}

}

$fn7 = $fn7_prep("hello")


$fn8_prep = func($sam str) { # this is bad because it's simpler to do it that way and technically $fn8 is static but the insides are not timeless

	if datetime.now() % 2 == 0 {

		func($x) { # middle point
			strconv.itoa($x)
		}

	} else {

		func($x) {
			"hello" + $sam
		}

	}

}

$fn8 = $fn8_prep(os.system("seq 10"))


$fn9_prep = func($sam int) { # we actually agree this one is okay, but the reason it's okay is because $fn9 changes for each datetime.now() timeful change.

	if $sam % 2 == 0 {

		func($x) { # middle point
			strconv.itoa($x)
		}

	} else {

		func($x) {
			"hello"
		}

	}

}

$fn9 = $fn9_prep(datetime.now())


test "test1" {
	anotherstr => "hello",
	func1 => $fn3, // txn!
}



#
#	WHY DO WE WANT THIS?
#

import "prometheus"

# prometheus.connect is not pure because it has some sneaky side-effect somewhere...

$conn = func($x string) int {
	prometheus.connect($x) + 42 # not pure but timeless
}

test "test2" {
	func1 => $conn,
}


import "flagthing" # a built-in library

$flag1 = "flag1" # some arbitrary name

if flagthing.happened($flag1) {
	print "wow" {}
}

test "test2" {
	flagname => $flag1,
	func1 => $flagthing.start,
}

@purpleidea
Copy link
Owner Author

@gelisam Just wanted to double-check if this patch with interface and one implementation is basically what you were expecting? If so, LMK and I can complete it for other functions too.

@gelisam
Copy link
Contributor

gelisam commented Oct 4, 2023

Looks good, yes!

@gelisam
Copy link
Contributor

gelisam commented Oct 7, 2023

Next steps:

  • Fill in the panic("TODO") in StmtFoo.TimeCheck()
  • Add a Timeless info field to Func, or just a Func.TimeCheck() method
  • Use that Func.TimeCheck to fill in the obj. Function case in ExprFunc.TimeCheck()
  • Fill in the obj.Values case in ExprFunc.TimeCheck()
  • convert a FuncValue to a golang function, either via Func.Call() or by sending a single value to the sub-graph and waiting for the result.

@gelisam
Copy link
Contributor

gelisam commented Oct 7, 2023

  • write tests exercising the new time check

@purpleidea purpleidea force-pushed the feat/send-func-to-res branch 5 times, most recently from 64fd35b to 73dfec6 Compare December 6, 2023 21:25
@purpleidea
Copy link
Owner Author

Not the priority, but I was playing with this code and implemented a few extra for fun. I also pushed in your commits to have them here.

An interesting realization I had is that we should eventually once all have been implemented, be able to remove each mostly identical custom Stream() implementation with a single one that is common to all functions. Or most functions!

Add a new interface for callable functions.
Add an initial implementation for the `if` function.
Add an initial implementation for the `const` function.
Add initial implementations for these wrapper functions.
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.

None yet

2 participants