flux.2-klein-4b enablement#36544
Conversation
github-actions
Bot
added
category: GPU
|
build_jenkins |
|
@mpaulitsch please resolve conflicts |
resolved all conflicts -- mainly in src/plugins/intel_gpu/src/plugin/transformations_pipeline.cpp |
| mark_path_from_output(m.get_pattern_value_map().at(pattern_root).get_node(), m_visited, skip_pred); | ||
| return true; | ||
| }; | ||
| register_matcher(std::make_shared<Matcher>(pattern_root, name), callback); |
There was a problem hiding this comment.
register_mark_root calls register_matcher once per pattern, and it's invoked 6× at lines 74-79. But MatcherPass holds a single m_matcher/m_handler — each call overwrites the previous, so only the last (ref_sin, :79) survives. The other five — including the primary pos_cos/pos_sin — are dead code. The PR's goal (keeping RoPE cos/sin tables in FP32) is not achieved.
| using namespace ov::pass::pattern; | ||
|
|
||
| // FLUX.2 pos_embed frequency tables: outer-product MatMul -> Cos / Sin. | ||
| auto outer_matmul = wrap_type<v0::MatMul>({any_input(), any_input()}); |
There was a problem hiding this comment.
wrap_type<v0::MatMul>({any_input(), any_input()}) is unconstrained → pos_cos/pos_sin match any MatMul→Cos/Sin and pull the whole MatMul subgraph into FP32. Currently inert because of the blocker at 71 line, but once it's fixed this becomes an accuracy/perf regression. Constrain it to the RoPE context (shape/symbol constraints, or match the formed ov::op::internal::RoPE).
| auto pos_sin = wrap_type<v0::Sin>({outer_matmul}); | ||
|
|
||
| // rotary_emb / LLM-style tables: Concat -> Cos or Sin (e.g. text encoder rotary_emb). | ||
| auto rot_concat = wrap_type<v0::Concat>({any_input(), any_input()}); |
There was a problem hiding this comment.
Same issue — Concat→Cos/Sin is too broad and will catch non-RoPE subgraphs.
|
|
||
| } // namespace | ||
|
|
||
| DisableFP16CompFlux2RoPEPattern::DisableFP16CompFlux2RoPEPattern() { |
There was a problem hiding this comment.
No test exists for this pass (the PR only adds tests in glu_fusion_test.cpp), which is why the blocker is invisible to CI. Add a graph-level test asserting is_conversion_disabled(cos/sin, f16) for all 6 patterns.
|
build_jenkins |
mpaulitsch
force-pushed
the
mpa/flux.2-klein-enablement
branch
from
41e53c5 to
2744594
Compare
|
thanks for review Alexander ... addressed comments |
| OPENVINO_ASSERT(pattern_map.count(axis_const_m)); | ||
| auto mul = ov::as_type_ptr<v1::Multiply>(pattern_map.at(mul_m).get_node_shared_ptr()); | ||
| if (!mul || transformation_callback(mul)) | ||
| const auto mul_node = pattern_map.at(mul_m).get_node_shared_ptr(); |
There was a problem hiding this comment.
Do we really need renaming changes?
mpaulitsch
force-pushed
the
mpa/flux.2-klein-enablement
branch
2 times, most recently
from
ddb7b32 to
301532c
Compare
|
rebaselined to be up-to-date with master branch. |
|
build_jenkins |
Match VariadicSplit with runtime Concat split_lengths and TypeRelaxed multiply, and run GLUFusion before f16 activation scaling and at PostLPT start so the pattern is not rewritten by FC or MoveDownScalarMul passes. Co-authored-by: Cursor <cursoragent@cursor.com>
Align MarkRopeInputsToKeepInMixedPrecision with disable_conversion(f16), add DisablePrecisionConversion rt_info (backed by legacy disable_fp16_compression wrappers), and run the pass on the GPU plugin after RoPEFusion so cos/sin subgraphs stay in FP32 under f16 compression. Co-authored-by: Cursor <cursoragent@cursor.com>
mpaulitsch
force-pushed
the
mpa/flux.2-klein-enablement
branch
from
301532c to
2cb23a0
Compare
|
build_jenkins |
4 participants
Details:
Tickets:
AI Assistance:
yes, created and checked output aligning with expectations, created extensive performance tests, code review.