Conversation
6c1ff68 to
4ac7a92
Compare
okofish
left a comment
There was a problem hiding this comment.
Another issue is that in order to construct the checkpoints file, we will need final states. I'm not returning the final states now as discussed, so we will need to reevaluate to generate that file.
I've moved away from my original idea of directly serializing structs in favor of manually constructing binary files, which isn't that hard anyway. So it's OK for you to return the states, and I may or may not do something with them.
I'm not clear on which dispositions should end up feeding into the master crib file. Is it just ReachedCycle?
| namespace PostTagSystem { | ||
| class PostTagSearcher { | ||
| public: | ||
| enum class TerminationReason { |
There was a problem hiding this comment.
Minor quibble: since the evaluation didn't necessarily terminate, this might be better called something like ConclusionReason or ConclusionDisposition.
| // Unlike tape length and event count constraints, time constraint applies to the entire range/group. | ||
| // If the time constraint is exceeded the current EvaluationResult and all the remaining ones will have | ||
| // TimeConstraintExceeded termination reason. The aborted evaluations will have EvaluationResult filled in with | ||
| // values obtained so far. The remaining ones will be filled with zeros. |
There was a problem hiding this comment.
Is this saying that evaluateRange(begin, end, ...) will always return a vector of end - begin + 1 EvaluationResults even if the evaluation timed out in the middle and half of the results were never evaluated? If so, this is OK, but I would like some way to distinguish the evaluation(s) that the timeout happened during, and hence do have some useful data inside, from the evaluations that were never reached at all. Could there be another TerminationReason to describe the latter case? (e.g. NotEvaluated)
| // If the time constraint is exceeded the current EvaluationResult and all the remaining ones will have | ||
| // TimeConstraintExceeded termination reason. The aborted evaluations will have EvaluationResult filled in with | ||
| // values obtained so far. The remaining ones will be filled with zeros. | ||
| uint64_t groupTimeConstraintNs; |
There was a problem hiding this comment.
Nanoseconds? Hmm, I guess that will be enough precision... :)
| // (trie is not thread-safe for writing) | ||
|
|
||
| // Computes results for states between begin and the one preceeding end. | ||
| // The head state is incremeneted first, and once it reaches 3, it is reset to zero with an incremented tape. |
There was a problem hiding this comment.
Should I always give a begin state with head state 0?
What about end; does it matter? Should it be 3 since in this ordering that's the last state?
There was a problem hiding this comment.
Never mind; I just saw the evaluateRange(uint64_t, ...) signature below which is presumably what I'll be using, so I don't need to worry about the raw state structs.
| const PostTagState& end, | ||
| const EvaluationParameters& parameters); | ||
|
|
||
| // Sets phases to zero and reads in tapeBegin and tapeEnd as binary digits. |
There was a problem hiding this comment.
How does the head work here? Is that the "phase" you mention or is that something else?
Do I need to somehow keep track of the initial head states accompanying initial conditions in order to feed them back into evaluateGroup for pounce ("long-haul") jobs?
| struct SmallState { | ||
| uint64_t tape; | ||
| uint8_t headState; | ||
| }; |
There was a problem hiding this comment.
Should this struct include a tapeLength?
| const EvaluationParameters& parameters); | ||
|
|
||
| // Sets phases to zero and reads in tapeBegin and tapeEnd as binary digits. | ||
| std::vector<EvaluationResult> evaluateRange(uint64_t tapeLength, |
There was a problem hiding this comment.
Could tapeLength safely be a uint8_t?
| uint64_t eventCount; | ||
| uint64_t maxTapeLength; | ||
| uint64_t finalTapeLength; | ||
| }; |
There was a problem hiding this comment.
It would be helpful for me if this struct included a copy of the initial state, either as a PostTagState/TagState or as a SmallState.
maxitg
left a comment
There was a problem hiding this comment.
Ok, I'll return the states.
If I understand the plan correctly, we will accumulate the list of states reached around tape length 1000 (±8). We then pass them to the crib file so that these states don't have to be evolved again.
So, I think we will want all dispositions other than ReachedKnownCheckpoint, which have maxTapeLength >= 1000. However, instead of passing the final state to the crib file, we will want all intermediate states with lengths between 1000 and 1008, which we'll have to compute offline (or with a different kind of job). Note that computing these intermediate states will require a different evolution code because the bit-packed version in PostTagHistory skips 8 steps at a time.
Also, I changed TerminationReason -> ConclusionReason, and added NotEvaluated reason. Note that evaluatedRange(begin, end, ...) returns the vector of length end - begin because it works like std iterators.
As for nanoseconds, hopefully, we can finish the project in 584.9 yrs :)
Reviewable status: 0 of 12 files reviewed, 5 unresolved discussions (waiting on @okofish and @sskini2)
libPostTagSystem/PostTagSearcher.hpp, line 28 at r1 (raw file):
Previously, okofish (Jesse Friedman) wrote…
It would be helpful for me if this struct included a copy of the initial state, either as a
PostTagState/TagStateor as aSmallState.
Done.
libPostTagSystem/PostTagSearcher.hpp, line 51 at r1 (raw file):
Previously, okofish (Jesse Friedman) wrote…
Never mind; I just saw the
evaluateRange(uint64_t, ...)signature below which is presumably what I'll be using, so I don't need to worry about the raw state structs.
Keep in mind, end works the same way as iterators. It will evaluate the states from begin to the predecessor of end. The same is true for the uint64_t signature..`
libPostTagSystem/PostTagSearcher.hpp, line 56 at r1 (raw file):
Previously, okofish (Jesse Friedman) wrote…
How does the head work here? Is that the "phase" you mention or is that something else?
Do I need to somehow keep track of the initial head states accompanying initial conditions in order to feed them back into
evaluateGroupfor pounce ("long-haul") jobs?
Yes, the head state is the same as the phase, and it is a part of the state, so you do need to keep track of them.
libPostTagSystem/PostTagSearcher.hpp, line 57 at r1 (raw file):
Previously, okofish (Jesse Friedman) wrote…
Could
tapeLengthsafely be auint8_t?
Good point, it can be in this case (not in other cases like the evaluation result).
libPostTagSystem/PostTagSearcher.hpp, line 69 at r1 (raw file):
Previously, okofish (Jesse Friedman) wrote…
Should this struct include a
tapeLength?
Yes, it should. Good catch!
|
I implemented most of the API except for max tape length, time constraint, and checkpoints parameters (which it ignores for now). @okofish, you can start using it already, or you can wait until I test it and implement the remaining parameters later tonight. Also, I don't mind merging it now if it's more convenient. |
|
@maxitg Seeing as the tests pass I think it would be helpful if you can merge this now and I'll rebase my branch onto master. Thanks! |
okofish
left a comment
There was a problem hiding this comment.
Reviewable status: 0 of 14 files reviewed, 3 unresolved discussions (waiting on @okofish and @sskini2)
Changes
PostTagSearcher.PostTagSearcherallows one to evaluate multiple inits at once for the Post system.Comments
One issue we have here is that
PostTagHistoryruns 8 events at a time, which means it will not go through every tape length (it can change by 8 in 8 events), so we need either something more complicated for computing checkpoints, or I need to be smarter about catching the checkpoints and switch to a slow evaluation mode near the checkpoints' size (which would make the code messier and less reliable).Another issue is that in order to construct the checkpoints file, we will need final states. I'm not returning the final states now as discussed, so we will need to reevaluate to generate that file.
Finally, the notebook mentions that we might want to return cycle periods. The bitpacked-system cycle periods can be inferred from the event count (by comparing it to the previous power of two). However, computing cycle periods precisely will require doing some slow evaluation.
@sskini2, and comments on the concerns above?
@okofish, could you take a look over the API and see if you can spot anything missing/inconvenient?
This change is