[bug-3878]The pctGood history aggregate function has an off-by-one error

I’ve been learning about custom history aggregate functions, and tried to implement my own pctGood. I came up with a version that perfectly matched the results of the built one every single time. Then I noticed that neither version ever returned 100%. Both my version and presumably the built in one work by accumulating milliseconds duration that the tag is “good”, then divide by the time window period which will always be one millisecond longer than the accumulator could possibly be.

I modified my version to return accumulated_ms / (ms_range - 1) and it started returning 100% perfectly.

I suppose this falls into the category of “errors so small no one ever cared”, but it bugs me.

Here’s my custom aggregator which returns 100% properly:

wrapper = '''python:
def myPctGood(qval, interpolated, finished, blockContext, queryContext):
	prior_state = blockContext.get('prior_state', None)
	prior_start_ms = blockContext.get('prior_start_ms', None)
	accumulated_ms = blockContext.get('accumulated_ms', 0)
	current_state = qval.quality.isGood()
	current_ms = system.date.toMillis(qval.timestamp)
	if current_state != prior_state:
		if prior_state:
			accumulated_ms += (current_ms - prior_start_ms)
			blockContext['accumulated_ms'] = accumulated_ms
		prior_state = current_state
		prior_start_ms = current_ms
		blockContext['prior_state'] = prior_state
		blockContext['prior_start_ms'] = prior_start_ms
	if finished:
		if current_state:
			accumulated_ms += (current_ms - prior_start_ms)
		return (float(accumulated_ms) / float(blockContext.blockEnd - blockContext.blockStart - 1))
'''

As an aside, I’m a bit horrified by how the python: custom aggregators are managed. I would feel safer if there were some way to pass in a pure Python lambda or function instead of a string that was compiled at runtime.

EDIT: I wouldn’t be surprised if other built-in history aggregators had the same issue as well.

1 Like

You should be able to pass a reference to a project script; see the 'Example' section:
https://docs.inductiveautomation.com/display/DOC81/Custom+Tag+History+Aggregates#CustomTagHistoryAggregates-Examples
(Due to a bug, you'll have to prefix with shared, but that's not a huge deal).

As for the main point of your post, I compared your black-box implementation against our codebase and it's basically identical, and looks like we are indeed subject to the bug you noticed. I filed a ticket.

1 Like

I had tried following the example for “shared” in the docs previously and was unable to get it to work. It just occurred to me that I haven’t tried putting my functions in a project library named “shared” yet. The example didn’t do that, but I’ve already found other things wrong with that document page. (I have a support ticket open.)

Yes. The examples in the docs are wrong, but if you just put your functions in some namespace that begins with the string “shared” they work.

And even the “shared” method is still passing a string that is compiled at runtime. I passed in “shared = 3 #” as my aggregate, and got this error in the wrapper log:

Error compiling custom python aggregate.
SyntaxError: ("mismatched input '=' expecting NEWLINE", ('<function:pyaggimpl>', 2, 15, '\treturn shared = 3 #(qval, interpolated, finished, blockContext, queryContext)\n'))

So, yeah, I’d still feel safer if there were a way to pass a pure Python lambda or function instead of a string that was compiled at runtime.

1 Like

You're right. I'm assuming this had something to do with the project system rework in 8.0, but it's been a while since I messed around with custom aggregates in 7.9 or prior.

Thanks for the heads up, and thanks for sending the other errors you found to the support team. I'll incorporate them into the page.

Edit: I should add, the bug PGriffith mentioned is still in play. Normally we don't document bugs, but since this feature is basically useless without naming the project library something like "shared", I'm happy to make an exception. Once we fix the bug I'll update the page again.

2 Likes

They don't serialize across the wire, so, not gonna happen.

1 Like

New docs look good! :+1:

I only started learning Ignition around the same time 8.0 came out. Did 7.9 and older use "shared." as a special namespace to reference scripts in a certain location? It doesn't do anything anymore as far as I can tell.

That’s the gist of it, yes. You used to have a single ‘global’ project which could house a few certain resource types, one of which was scripts, and referencing them anywhere else required the shared. prefix.