-
Notifications
You must be signed in to change notification settings - Fork 9
Update documentation for Global and clean up usage of globalStatus #187
Conversation
This changes the wording somewhat from the Local documentation, I can rectify the differences when we like the Global wording. |
global.go
Outdated
} | ||
|
||
// globalOperation executes the requested operation at the given location. | ||
// Modifications to this function must keep in mind that it can be called |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Modifications to this function must keep in mind that it..."
this reads a bit funny.
perhaps:
Modifications to this function must be applied while keeping in mind that globalOperation can be...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
// Done communicates to the optimization method that the optimization has | ||
// concluded to allow for shutdown. | ||
|
||
// Done communicates that the optimization has concluded to allow for shutdown. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps semantically relate it to the sync.WaitGroup.Done()
method? (or explicitly unrelate it to that)
I was a bit thrown off tracks that it didn't return anything (not even struct{}
) so it couldn't really be used like in:
for {
select {
case <-gm.Done(): // do clean-up
}
}
and then I remembered that it looked a bit like sync.WaitGroup.Done()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've returned to this after a long time, some comments are old, some are new. Read them accordingly.
global.go
Outdated
// operation, the requested fields of Problem will be evaluated at the value | ||
// in Location.X, filling the corresponding fields of Location. These values | ||
// can be retrieved and used upon the next call to IterateGlobal with that task id. | ||
// The corresponding fields of Location will be filled with the results that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete this line
global.go
Outdated
// aside from the commanded evaluations. Thus, the type implementing Method may | ||
// use multiple Operations to set the Location fields at a particular x value. | ||
// | ||
// A GlobalMethod may alternately declare MajorIteration. This updates the optimal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rewrite this paragraph to something like:
When IterateGlobal declares MajorIteration, the caller can update the optimal location
to the values in Location, and check for convergence. The type implementing
GlobalMethod must make sure that the fields of Location are valid and consistent.
global.go
Outdated
// location to the values in Location, and checks for the convergence of the | ||
// optimization. Consequently, the fields of Location must be valid and consistent. | ||
// | ||
// A GlobalMethod must not return InitIteration and PostIteration operations. These are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/A GlobalMethod/IterateGlobal/
// optimization. Consequently, the fields of Location must be valid and consistent. | ||
// | ||
// A GlobalMethod must not return InitIteration and PostIteration operations. These are | ||
// reserved for the clients to be passed to Recorders. A Method must also not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/A Method/IterateGlobal/
global.go
Outdated
// can be retrieved and used upon the next call to IterateGlobal with that task id. | ||
// The corresponding fields of Location will be filled with the results that | ||
// The GlobalMethod interface requires that entries of Location are not modified | ||
// aside from the commanded evaluations. Thus, the type implementing Method may |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Method/GlobalMethod/
global.go
Outdated
// It uses a mutex to protect updates where necessary. | ||
func (g *globalStatus) globalOperation(op Operation, loc *Location, x []float64) Status { | ||
// Do a quick check to see if one of the other workers converged in the meantime. | ||
// globalStatus is a data struct that coordinates information between concurrently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
globalStatus coordinates access to information shared between ...
?
global.go
Outdated
|
||
// updateStatus updates the status and error fields of g. This update only happens | ||
// if status == NotTerminated, so that the first different status is the one | ||
// maintained |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dot at the end.
global.go
Outdated
// globalOperation executes the requested operation at the given location. | ||
// When modifying this function, keep in mind that it can be called concurrently. | ||
// Uses of the internal fields should be through the methods of globalStatus and | ||
// protected by a mux where appropriate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mutex
global.go
Outdated
switch op { | ||
case NoOperation: | ||
case InitIteration: | ||
panic("optimize: Method returned InitIteration") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GlobalMethod
global.go
Outdated
case InitIteration: | ||
panic("optimize: Method returned InitIteration") | ||
case PostIteration: | ||
panic("optimize: Method returned PostIteration") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GlobalMethod
Done on all requests. |
Implemented by gonum/gonum#265 |
No description provided.