Skip to content

Commit af91c73

Browse files
galvgalv
committed
hmm-utils.cc compiles, except for a bizarre problem with the copy-assignment operator.
1 parent a8b5580 commit af91c73

4 files changed

Lines changed: 103 additions & 53 deletions

File tree

‎src/chain/chain-supervision.cc‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ bool TimeEnforcerFst::GetArc(StateId s, Label ilabel, fst::StdArc* oarc) {
240240
oarc->ilabel = ilabel;
241241
if (convert_to_pdfs_) {
242242
// the olabel will be a pdf-id plus one, not a transition-id.
243-
int32 pdf_id = trans_model_.TransitionIdToPdf(ilabel);
243+
int32 pdf_id = trans_model_.TransitionIdToPdfFast(ilabel);
244244
oarc->olabel = pdf_id + 1;
245245
} else {
246246
oarc->olabel = ilabel;
@@ -276,7 +276,7 @@ bool TrainingGraphToSupervisionE2e(
276276
}
277277
KALDI_ASSERT(arc.ilabel != 0);
278278
StdArc arc2(arc);
279-
arc2.ilabel = arc2.olabel = trans_model.TransitionIdToPdf(arc.ilabel) + 1;
279+
arc2.ilabel = arc2.olabel = trans_model.TransitionIdToPdfFast(arc.ilabel) + 1;
280280
aiter.SetValue(arc2);
281281
}
282282
}
@@ -337,11 +337,11 @@ bool ProtoSupervisionToSupervision(
337337
// when we compose with the denominator graph.
338338
h_cfg.transition_scale = 0.0;
339339

340-
VectorFst<StdArc> *h_fst = GetHTransducer(inv_cfst.IlabelInfo(),
341-
ctx_dep,
342-
trans_model,
343-
h_cfg,
344-
&disambig_syms_h);
340+
std::unique_ptr<VectorFst<StdArc>> h_fst = GetHTransducer(inv_cfst.IlabelInfo(),
341+
ctx_dep,
342+
trans_model,
343+
h_cfg,
344+
&disambig_syms_h);
345345
KALDI_ASSERT(disambig_syms_h.empty());
346346

347347
VectorFst<StdArc> transition_id_fst;
@@ -352,10 +352,7 @@ bool ProtoSupervisionToSupervision(
352352
// when we compose with the denominator graph.
353353
BaseFloat self_loop_scale = 0.0;
354354

355-
// You should always set reorder to true; for the current chain-model
356-
// topologies, it will affect results if you are inconsistent about this.
357-
bool reorder = true,
358-
check_no_self_loops = true;
355+
bool check_no_self_loops = true;
359356
// add self-loops to the FST with transition-ids as its labels.
360357
AddSelfLoops(trans_model, disambig_syms_h, self_loop_scale, reorder,
361358
check_no_self_loops, &transition_id_fst);

‎src/fstext/fstext-utils-inl.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,6 @@ void MakePrecedingInputSymbolsSameClass(MutableFst<Arc> *fst, const F &f) {
539539
typedef typename Arc::Weight Weight;
540540
vector<ClassType> classes;
541541
ClassType noClass = f(kNoLabel);
542-
ClassType epsClass = f(0);
543542

544543
// Find bad states (states with multiple input-symbols into them).
545544
std::set<StateId> bad_states; // states that we need to change.

‎src/hmm/hmm-utils.cc‎

Lines changed: 87 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,42 @@ GetPdfToTransitionIdTransducer(const Transitions &trans_model) {
332332
return ans;
333333
}
334334

335+
struct TransitionState {
336+
public:
337+
TransitionState(const Transitions::TransitionIdInfo& info):
338+
info(info) { }
339+
340+
bool operator==(const TransitionState& other) const {
341+
return info.phone == other.info.phone &&
342+
info.topo_state == other.info.topo_state &&
343+
info.pdf_id == other.info.pdf_id;
344+
}
335345

346+
bool operator!=(const TransitionState& other) const {
347+
return !(*this == other);
348+
}
349+
350+
TransitionState& operator=(TransitionState other) {
351+
// TODO: Fix this bizarre error when I uncomment this:
352+
// this->info = other.info;
353+
KALDI_ASSERT(false);
354+
// hmm-utils.cc: In member function ‘kaldi::TransitionState& kaldi::TransitionState::operator=(kaldi::TransitionState)’:
355+
// hmm-utils.cc:351:24: error: passing ‘const kaldi::Transitions::TransitionIdInfo’ as ‘this’ argument discards qualifiers [-fpermissive]
356+
// this->info = other.info;
357+
// ^~~~
358+
// In file included from ../hmm/hmm-utils.h:27:0,
359+
// from hmm-utils.cc:25:
360+
// ../hmm/transitions.h:107:10: note: in call to ‘kaldi::Transitions::TransitionIdInfo& kaldi::Transitions::TransitionIdInfo::operator=(const kaldi::Transitions::TransitionIdInfo&)’
361+
362+
return *this;
363+
}
364+
365+
bool operator<(const TransitionState& other) const {
366+
return info < other.info;
367+
}
368+
369+
const Transitions::TransitionIdInfo& info;
370+
};
336371

337372
class TidToTstateMapper {
338373
public:
@@ -358,9 +393,12 @@ class TidToTstateMapper {
358393
bool check_no_self_loops):
359394
trans_model_(trans_model),
360395
disambig_syms_(disambig_syms),
361-
check_no_self_loops_(check_no_self_loops) { }
396+
check_no_self_loops_(check_no_self_loops) {
397+
KALDI_ASSERT((*this)(fst::kNoLabel) == NoLabelClass());
398+
KALDI_ASSERT((*this)(0) == ZeroClass());
399+
}
362400

363-
typedef Transitions::TransitionIdInfo Result;
401+
typedef TransitionState Result;
364402
static const Result& NoLabelClass() {
365403
// Take advantage of the fact that phone must be greater than or
366404
// equal to 1 to create a TransitionIdInfo which in practice will
@@ -370,24 +408,27 @@ class TidToTstateMapper {
370408
// whether we are using one of these invalid TransitionIdInfo
371409
// classes.
372410
static auto *no_label =
373-
new Transitions::TransitionIdInfo{.phone = -1, .topo_state = -1, .arc_index = -1,
374-
.pdf_id = -1, .self_loop_pdf_id = -1};
375-
return *no_label;
411+
new Transitions::TransitionIdInfo{.phone = -1, .topo_state = -1,
412+
.arc_index = -1, .pdf_id = -1,
413+
.self_loop_pdf_id = -1};
414+
static auto *no_label_state = new TransitionState(*no_label);
415+
return *no_label_state;
376416
}
377417

378418
static const Result& ZeroClass() {
379419
static auto *zero_label =
380420
new Transitions::TransitionIdInfo{.phone = 0, .topo_state = -1, .arc_index = -1,
381421
.pdf_id = -1, .self_loop_pdf_id = -1};
382-
return *zero_label;
422+
static auto *zero_label_state = new TransitionState(*zero_label);
423+
return *zero_label_state;
383424
}
384425

385-
const Result& operator() (int32 tid) const {
426+
Result operator() (int32 tid) const {
386427
if (tid == static_cast<int32>(fst::kNoLabel)) return NoLabelClass(); // -1 -> -1
387428
else if (tid >= 1 && tid <= trans_model_.NumTransitionIds()) {
388429
if (check_no_self_loops_ && trans_model_.InfoForTransitionId(tid).is_self_loop)
389430
KALDI_ERR << "AddSelfLoops: graph already has self-loops.";
390-
return trans_model_.InfoForTransitionId(tid);
431+
return TransitionState(trans_model_.InfoForTransitionId(tid));
391432
} else { // 0 or (presumably) disambiguation symbol. Map to zero
392433
int32 big_number = fst::kNontermBigNumber; // 1000000
393434
if (tid != 0 && tid < big_number)
@@ -404,6 +445,21 @@ class TidToTstateMapper {
404445
bool check_no_self_loops_;
405446
};
406447

448+
// Returns true if the outgoing arcs of the state s sum to 1.0
449+
template<typename FST>
450+
static bool StateIsStochastic(FST fst, typename FST::StateId s) {
451+
using namespace fst;
452+
using Arc = typename FST::Arc;
453+
using Weight = typename Arc::Weight;
454+
Weight total_prob = Weight::Zero();
455+
for (MutableArcIterator<MutableFst<Arc> > aiter(fst, s);
456+
!aiter.Done();
457+
aiter.Next()) {
458+
total_prob = Plus(total_prob, aiter.Value());
459+
}
460+
return ApproxEqual(total_prob.Value(), Weight::One());
461+
}
462+
407463
// This is the code that expands an FST from transition-states to
408464
// transition-ids, in the case where the non-optional transition is before
409465
// the self-loop.
@@ -417,8 +473,6 @@ static void AddSelfLoopsInternal(const Transitions &trans_model,
417473
typedef Arc::StateId StateId;
418474
typedef Arc::Weight Weight;
419475

420-
typedef TidToTstateMapper::Result Class;
421-
422476
TidToTstateMapper f(trans_model, disambig_syms, check_no_self_loops);
423477
// Duplicate states as necessary so that each state will require at most one
424478
// self-loop to be added to it. Approximately this means that if a
@@ -430,11 +484,12 @@ static void AddSelfLoopsInternal(const Transitions &trans_model,
430484
// into each state. This works because each state now has only one
431485
// transition state coming into (because of
432486
// MakePrecedingInputSymbolsSameClass).
433-
std::vector<Class> state_in(fst->NumStates(), f.NoLabelClass());
487+
std::vector<TransitionState> state_in(fst->NumStates(), f.NoLabelClass());
434488

435489
// This first loop just works out the label into each state,
436490
// and converts the transitions in the graph from transition-states
437491
// to transition-ids.
492+
// state_in maps each state in the fst to its TransitionState
438493

439494
for (StateIterator<VectorFst<Arc> > siter(*fst);
440495
!siter.Done();
@@ -444,7 +499,7 @@ static void AddSelfLoopsInternal(const Transitions &trans_model,
444499
!aiter.Done();
445500
aiter.Next()) {
446501
const Arc& arc = aiter.Value();
447-
Class trans_state = f(arc.ilabel);
502+
TransitionState trans_state = f(arc.ilabel);
448503
if (state_in[arc.nextstate] == f.NoLabelClass()) {
449504
state_in[arc.nextstate] = trans_state;
450505
} else {
@@ -454,10 +509,8 @@ static void AddSelfLoopsInternal(const Transitions &trans_model,
454509
}
455510
}
456511

457-
// state_in maps each state in the fst to its TransitionState
458-
KALDI_ASSERT(state_in[fst->Start()] == f.NoLabelClass() ||
459-
state_in[fst->Start()] == f.ZeroClass());
460-
// or MakePrecedingInputSymbolsSame failed.
512+
// The start state should have no incoming arcs (invariant of Topology)
513+
KALDI_ASSERT(state_in[fst->Start()] == f.ZeroClass());
461514

462515
// The next loop looks at each graph state, adds the self-loop [if needed] and
463516
// multiples all the out-transitions' probs (and final-prob) by the
@@ -466,36 +519,32 @@ static void AddSelfLoopsInternal(const Transitions &trans_model,
466519
// with the corresponding labels on them by this probability).
467520

468521
for (StateId s = 0; s < static_cast<StateId>(state_in.size()); s++) {
469-
if (state_in[s] != f.NoLabelClass() && state_in[s] != f.ZeroClass()) {
522+
const TransitionState& trans_state = state_in[s];
523+
if (trans_state != f.NoLabelClass() && trans_state != f.ZeroClass() &&
524+
trans_state.info.self_loop_pdf_id != -1) {
470525
// defined, and not eps or a disambiguation symbol or a
471-
// nonterminal-related sybol for grammar decoding...
472-
const Class& trans_state = state_in[s];
473-
// First multiply all probabilities by "forward" probability.
474-
475-
// WARNING: This is no longer the forward probability that this code was originally using!
476-
// It is difficult to get the self-loop probability just given the
477-
trans_model.InfoForTransitionId(trans_state.self_loop_pdf_id).transition_cost;
478-
BaseFloat log_prob = trans_state.transition_cost;
479-
fst->SetFinal(s, Times(fst->Final(s), Weight(-log_prob)));
526+
// nonterminal-related symbol for grammar decoding, and has a
527+
// self-loop which needs to be added, while maintaining
528+
int32 self_loop_tid = trans_state.info.self_loop_transition_id;
529+
KALDI_ASSERT(self_loop_tid != 0 &&
530+
"Can't have a self_loop_pdf_id without a self_loop_transition_id");
531+
// 1) Multiply all probabilities by "forward" probability.
532+
BaseFloat self_loop_log_prob =
533+
-trans_model.InfoForTransitionId(self_loop_tid).transition_cost;
534+
BaseFloat log_forward_prob = log(1.0 - exp(self_loop_log_prob));
535+
fst->SetFinal(s, Times(fst->Final(s), Weight(-log_forward_prob)));
480536
for (MutableArcIterator<MutableFst<Arc> > aiter(fst, s);
481537
!aiter.Done();
482538
aiter.Next()) {
483539
Arc arc = aiter.Value();
484-
arc.weight = Times(arc.weight, Weight(-log_prob));
540+
arc.weight = Times(arc.weight, Weight(-log_forward_prob));
485541
aiter.SetValue(arc);
486542
}
487-
// Now add self-loop, if needed.
488-
int32 trans_id = trans_state.self_loop_transition_id;
489-
// TODO: This is adding an arc for the current state into
490-
// itself, right? How are we supposed to get the current state's
491-
// self-loop? Ugh. And make sure I don't repeat it...
492-
if (trans_id != 0) { // has self-loop.
493-
auto&& trans_info = trans_model.InfoForTransitionId(trans_id).transition_cost;
494-
StateId next_state = TODO;
495-
BaseFloat log_prob = trans_model.InfoForTransitionId(trans_id).transition_cost;
496-
fst->AddArc(next_state, Arc(trans_id, 0, Weight(-log_prob), next_state));
497-
}
543+
// 2) Add self-loop
544+
fst->AddArc(s, Arc(self_loop_tid, 0, Weight(-self_loop_log_prob), s));
545+
498546
}
547+
KALDI_PARANOID_ASSERT(StateIsStochastic(fst, s));
499548
}
500549
}
501550

‎src/hmm/transitions.h‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ class Transitions {
159159
}
160160
// TODO. operator == can compare all members. Also compare derived members?
161161
bool operator == (const TransitionIdInfo &other) const {
162+
// I don't think this is being used right now. For now, just abort
163+
// whenever it is used, so I can see where it is used.
164+
KALDI_ASSERT(false);
162165
if (phone == other.phone && topo_state == other.topo_state &&
163166
pdf_id == other.pdf_id) {
164167
// This assertion is no longer true. Two states can have
@@ -173,9 +176,11 @@ class Transitions {
173176
return false;
174177
}
175178
}
176-
bool operator != (const TransitionIdInfo &other) const {
177-
return !(*this == other);
178-
}
179+
180+
// TransitionIdInfo& operator=(const TransitionIdInfo& other) {
181+
// is_final = other.is_final;
182+
// return *this;
183+
// }
179184
};
180185

181186

0 commit comments

Comments
 (0)