diff --git a/dev_notes/todo.txt b/dev_notes/todo.txt index f6926a4..78fdd68 100644 --- a/dev_notes/todo.txt +++ b/dev_notes/todo.txt @@ -14,7 +14,6 @@ We need an all_in_flux and all_out_flux or similar. For instance, SimplyP could be broken if it is used with soil discharge along a connection since it uses out_flux (and it can't know about the connections). - If an input series is provided indexed over "subcatchment", but should be indexed over "water body" and "subcatchment" is a union member of "water body", that should be made to work. (and all similar cases) Just re-map the index. If you load different modules using the same load name, you don't get a name clash! This is as intended if the two loads are identical (so that you can extend two models that load the same module) @@ -67,9 +66,6 @@ Lots of duplication between the different airsea things in general. The FPV versions should be able to reuse some of what is in the plain versions without code duplication. - The combining of phytoplankton module for NIVAFjord and EasyLake does not work that well in the Vansjø setup - Due to having made certain assumptions about N and C balance of these. Too much POC and PON from phyto. - NIVAFjord Vertical wind mixing using B-V. See page 45 in report 4. @@ -94,11 +90,6 @@ Sulfate reduction? Zooplankton? Experiment with different phytoplankton formulation. - - EasyChem - - Shading effect on phyto light availability (light should be more like an average in the epilimnion taking into account attenuation). - But that would also be problematic since it is quite uneven but the resulting phyto conc isn't. Connection system @@ -117,6 +108,9 @@ graph connections Regex + Maybe just remove this feature since it is too complex for the value we get out of it. + Just keep the necessary parts like checking what compartments are allowed and no cycles in case of no_cycles. + Finish regex check in the general case (cycles case) Regex checking of fjord_horizontal regex is bugged for nivafjord_moss @@ -180,6 +174,8 @@ *** Intermediate-pri *** + + If an input series is provided indexed over "subcatchment", but should be indexed over "water body" and "subcatchment" is a union member of "water body", that should be made to work. (and all similar cases) Just re-map the index. Store name of expected model file in data sets (?) Done now as a comment, for documentation purposes only. @@ -244,7 +240,11 @@ Multiple return values from functions (tuples). Needed if we want to implement MAGIC entirely in Mobius2 code for instance. Only annoying thing is that we then have to introduce 'tuple(N)' to the type system and check against it everywhere it is not allowed. A possible solution is to force it to be unpacked immediately at the call location of the function. I.e. if a function returns a tuple, you can only call it using something like - a, b := fun(), + a; b := fun(), + which de-sugars internally to + t := fun() + a := unpack(t, 0) + b := unpack(t, 1) Position_Map There can be problems if the max depth doesn't exactly match a boundary of two widths. diff --git a/models/example_data/NIVAFjord/glacier_lake_example.dat b/models/example_data/NIVAFjord/glacier_lake_example.dat index daef149..ca2b62d 100644 --- a/models/example_data/NIVAFjord/glacier_lake_example.dat +++ b/models/example_data/NIVAFjord/glacier_lake_example.dat @@ -522,7 +522,7 @@ data_set { par_group("Light") { par_real("Diffuse attenuation coefficent (clear water)") - [ 0.15 ] + [ 0.4 ] par_real("Shading factor") [ 0.003 ] diff --git a/src/ast.cpp b/src/ast.cpp index c8ec23c..83840e9 100644 --- a/src/ast.cpp +++ b/src/ast.cpp @@ -706,6 +706,25 @@ parse_math_block(Token_Stream *stream) { stream->expect_token(','); local_var->exprs.push_back(expr); block->exprs.push_back(local_var); + } else if (token.type == Token_Type::identifier && ((char)token2.type == ';')) { + auto unpack = new Unpack_Tuple_AST(); + unpack->names.push_back(token); + stream->read_token(); stream->read_token(); + while(true) { + token = stream->expect_token(Token_Type::identifier); + token2 = stream->read_token(); + unpack->names.push_back(token); + if(token2.type == Token_Type::def) + break; + else if((char)token2.type != ';') { + token2.print_error_header(); + fatal_error("Expected a ';' to continue the naming of tuple elements, or a ':=' for the assignment."); + } + } + auto expr = parse_math_expr(stream); + stream->expect_token(','); + unpack->exprs.push_back(expr); + block->exprs.push_back(unpack); } else { auto expr = parse_potential_if_expr(stream); block->exprs.push_back(expr); diff --git a/src/ast.h b/src/ast.h index 319c041..9fc2be4 100644 --- a/src/ast.h +++ b/src/ast.h @@ -190,6 +190,13 @@ Iterate_AST : Math_Expr_AST { Iterate_AST() : Math_Expr_AST(Math_Expr_Type::iterate) {}; }; +// TODO: should it be unified with Local_Var_AST ? +struct +Unpack_Tuple_AST : Math_Expr_AST { + std::vector names; + Unpack_Tuple_AST() : Math_Expr_AST(Math_Expr_Type::unpack_tuple) {}; +}; + struct Regex_Body_AST : Body_AST { Math_Expr_AST *expr; diff --git a/src/common_types.h b/src/common_types.h index 97eec3e..02a6cb2 100644 --- a/src/common_types.h +++ b/src/common_types.h @@ -121,7 +121,7 @@ name(Decl_Type type) { enum class Value_Type : s32 { - unresolved = 0, none, iterate, real, integer, boolean, + unresolved = 0, none, iterate, tuple, real, integer, boolean, }; inline bool is_value(Value_Type type) { return (s32)type >= (s32)Value_Type::real; } @@ -134,6 +134,7 @@ name(Value_Type type) { if(type == Value_Type::integer) return "integer"; if(type == Value_Type::boolean) return "boolean"; if(type == Value_Type::iterate) return "iterate"; + if(type == Value_Type::tuple) return "tuple"; return "unresolved"; } diff --git a/src/function_tree.cpp b/src/function_tree.cpp index 4e12cc4..2dc4720 100644 --- a/src/function_tree.cpp +++ b/src/function_tree.cpp @@ -92,7 +92,7 @@ add_local_var(Math_Block_FT *scope, Math_Expr_FT *val) { // NOTE: this should only be called on a block that is under construction. auto local = new Local_Var_FT(); local->exprs.push_back(val); - local->value_type = val->value_type; + local->value_type = Value_Type::none;//val->value_type; local->is_used = true; s32 id = scope->n_locals; local->id = id; @@ -270,18 +270,83 @@ void fixup_intrinsic(Function_Call_FT *fun, Token *name) { } } +Tuple_FT * +find_tuple(Math_Expr_FT *tuple) { + if(tuple->expr_type == Math_Expr_Type::block) + return find_tuple(tuple->exprs.back()); + else if(tuple->expr_type == Math_Expr_Type::tuple) + return static_cast(tuple); + tuple->source_loc.print_error_header(); + fatal_error(Mobius_Error::internal, "Unable to find tuple referenced by value."); + return nullptr; +} + +void +resolve_add_expr(Math_Expr_FT *parent, Math_Expr_FT *child, Standardized_Unit &unit, Function_Scope *scope, std::vector &units) { + + // If we get an "unpack tuple" we desugar it to a local var holding the tuple and other local vars accessing the elements of that tuple. + // TODO: Move this to a separate function to make it cleaner? + if(child->expr_type == Math_Expr_Type::unpack_tuple) { + // TODO: Assert the types are correct? + auto unpack = static_cast(child); + auto tuple = find_tuple(unpack->exprs[0]); + + if(unpack->names.size() != tuple->exprs.size()) { + unpack->source_loc.print_error_header(); + fatal_error("Incorrect number of elements in tuple unpacking. Expected ", tuple->exprs.size(), "."); + } + + auto tuple_local = new Local_Var_FT(); + tuple_local->name = "_tuple_"; // This should only be relevant for debug printing. Maybe make a better name?? + tuple_local->source_loc = unpack->source_loc; + tuple_local->value_type = Value_Type::none;//Value_Type::tuple; + tuple_local->is_used = true; // Protect it from being removed. + tuple_local->exprs.push_back(unpack->exprs[0]); + + Standardized_Unit no_unit = {}; + resolve_add_expr(parent, tuple_local, no_unit, scope, units); + // The id of the tuple local should now have been resolved by the nested call to resolve_add_expr above. + Local_Var_Id tuple_id = { scope->block->unique_block_id, tuple_local->id }; + + for(int idx = 0; idx < tuple->exprs.size(); ++idx) { + auto local = new Local_Var_FT(); + auto access = new Access_Tuple_Element_FT(); + + access->source_loc = unpack->source_loc; + access->value_type = tuple->exprs[idx]->value_type; + access->element_index = idx; + access->tuple_id = tuple_id; + + local->source_loc = unpack->source_loc; + local->value_type = Value_Type::none;//access->value_type; + local->name = unpack->names[idx]; + local->exprs.push_back(access); + + resolve_add_expr(parent, local, tuple->element_units[idx], scope, units); + } + + unpack->exprs.clear(); // So that the tuple itself is not deleted recursively. + delete unpack; // This one is not used in itself. + + return; + } + + parent->exprs.push_back(child); + if(child->expr_type == Math_Expr_Type::local_var) { + auto local = static_cast(child); + local->id = scope->block->n_locals++; + scope->local_var_units[local->id] = unit; + } + units.push_back(unit); +} + void resolve_arguments(Math_Expr_FT *ft, Math_Expr_AST *ast, Function_Resolve_Data *data, Function_Scope *scope, std::vector &units) { //TODO allow error check on expected number of arguments for(auto arg : ast->exprs) { auto result = resolve_function_tree(arg, data, scope); - ft->exprs.push_back(result.fun); - if(result.fun->expr_type == Math_Expr_Type::local_var) { - auto local = static_cast(result.fun); - local->id = scope->block->n_locals++; - scope->local_var_units[local->id] = result.unit; - } - units.push_back(std::move(result.unit)); + + resolve_add_expr(ft, result.fun, result.unit, scope, units); } } @@ -578,7 +643,7 @@ arguments_must_be_values(Math_Expr_FT *expr, Function_Scope *scope) { for(auto arg : expr->exprs) { if(!is_value(arg->value_type)) { arg->source_loc.print_error_header(); - error_print("This expression argument must resolve to a value."); + error_print("This expression argument must resolve to a value, not '", name(arg->value_type), "'."); fatal_error_trace(scope); } } @@ -617,6 +682,7 @@ resolve_special_directive(Function_Call_AST *ast, Directive directive, Function_ std::vector arg_units; resolve_arguments(new_fun, ast, data, scope, arg_units); + int allowed_arg_count = 1; if(directive == Directive::in_flux || directive == Directive::out_flux) allowed_arg_count = 2; @@ -727,6 +793,17 @@ resolve_special_directive(Function_Call_AST *ast, Directive directive, Function_ result.unit = std::move(arg_units[var_idx]); } +void +resolve_tuple(Function_Call_AST *ast, Function_Resolve_Data *data, Function_Scope *scope, Function_Resolve_Result &result) { + + auto new_tuple = new Tuple_FT(); + new_tuple->source_loc = ast->source_loc; + new_tuple->value_type = Value_Type::tuple; + resolve_arguments(new_tuple, ast, data, scope, new_tuple->element_units); + + result.fun = new_tuple; +} + void resolve_function_call(Function_Call_AST *fun, Function_Resolve_Data *data, Function_Scope *scope, Function_Resolve_Result &result) { @@ -824,7 +901,7 @@ resolve_function_call(Function_Call_AST *fun, Function_Resolve_Data *data, Funct auto inlined_arg = new Local_Var_FT(); inlined_arg->exprs.push_back(arg); inlined_arg->name = fun_decl->args[argidx]; - inlined_arg->value_type = arg->value_type; + inlined_arg->value_type = Value_Type::none;//arg->value_type; inlined_fun->exprs[argidx] = inlined_arg; inlined_arg->id = argidx; new_scope.local_var_units[argidx] = arg_units[argidx]; @@ -1146,6 +1223,8 @@ resolve_function_tree(Math_Expr_AST *ast, Function_Resolve_Data *data, Function_ auto directive = get_special_directive(fun->name.string_value); if(directive != Directive::none) resolve_special_directive(fun, directive, data, scope, result); + else if(fun->name.string_value == "tuple") + resolve_tuple(fun, data, scope, result); else resolve_function_call(fun, data, scope, result); @@ -1245,11 +1324,18 @@ resolve_function_tree(Math_Expr_AST *ast, Function_Resolve_Data *data, Function_ if(new_type == Value_Type::real) value_type = Value_Type::real; else if(new_type == Value_Type::integer && value_type == Value_Type::boolean) value_type = Value_Type::integer; + + if(new_type == Value_Type::tuple) { + new_if->exprs[idx]->source_loc.print_error_header(); + error_print("The values in an 'if' expression can't be tuples currently. Instead, create the elements using separate if expressions, then pack them into a tuple after."); + fatal_error_trace(scope); + } } if(value_type == Value_Type::iterate || value_type == Value_Type::none) { ifexpr->source_loc.print_error_header(); error_print("At least one of the possible results of the 'if' expression must evaluate to a value."); + fatal_error_trace(scope); } // Cast all possible result values up to the same type @@ -1413,6 +1499,26 @@ resolve_function_tree(Math_Expr_AST *ast, Function_Resolve_Data *data, Function_ result.unit = std::move(to_unit); } break; + case Math_Expr_Type::unpack_tuple : { + auto unpack = static_cast(ast); + auto new_unpack = new Unpack_Tuple_FT(); + new_unpack->value_type = Value_Type::none; + + std::vector arg_units; + resolve_arguments(new_unpack, ast, data, scope, arg_units); + + if(new_unpack->exprs[0]->value_type != Value_Type::tuple) { + ast->source_loc.print_error_header(); + fatal_error("Tried to unpack something that is not a tuple."); + } + + for(auto &name : unpack->names) + new_unpack->names.push_back(name.string_value); + + result.fun = new_unpack; + + } break; + default : { fatal_error(Mobius_Error::internal, "Unhandled math expr type in resolve_function_tree()."); } break; @@ -1507,6 +1613,14 @@ copy(Math_Expr_FT *source) { result = copy_one(source); } break; + case Math_Expr_Type::tuple : { + result = copy_one(source); + } break; + + case Math_Expr_Type::access_tuple_element : { + result = copy_one(source); + } break; + default : { fatal_error(Mobius_Error::internal, "Unhandled math expr type in copy()."); } break; @@ -1797,6 +1911,22 @@ print_tree_helper(Math_Expr_FT *expr, Print_Tree_Context *context, Print_Scope * //} } break; + case Math_Expr_Type::tuple : { + os << "tuple("; + int idx = 0; + for(auto arg : expr->exprs) { + print_tree_helper(arg, context, scope, block_tabs); + if (idx++ != expr->exprs.size()-1) os << ", "; + } + os << ")"; + } break; + + case Math_Expr_Type::access_tuple_element : { + auto access = static_cast(expr); + os << find_local_var(scope, access->tuple_id); + os << "[" << access->element_index << "]"; + } break; + case Math_Expr_Type::no_op : { os << "(no-op)"; } break; diff --git a/src/function_tree.h b/src/function_tree.h index 4dc580e..fb9fa61 100644 --- a/src/function_tree.h +++ b/src/function_tree.h @@ -169,6 +169,28 @@ Iterate_FT : Math_Expr_FT { Iterate_FT() : Math_Expr_FT(Math_Expr_Type::iterate) {} }; +struct +Tuple_FT : Math_Expr_FT { + // Unfortunately we have to store the units here even though they are only used during the resolution step. + std::vector element_units; + Tuple_FT() : Math_Expr_FT(Math_Expr_Type::tuple) {} +}; + +struct +Unpack_Tuple_FT : Math_Expr_FT { + std::vector names; + Unpack_Tuple_FT() : Math_Expr_FT(Math_Expr_Type::unpack_tuple) {} +}; + +struct +Access_Tuple_Element_FT : Math_Expr_FT { + s32 element_index = -1; + Local_Var_Id tuple_id; + + std::vector tuple_types; + Access_Tuple_Element_FT() : Math_Expr_FT(Math_Expr_Type::access_tuple_element) {} +}; + typedef std::unique_ptr owns_code; @@ -282,6 +304,9 @@ Math_Expr_FT * copy(Math_Expr_FT *source); +Tuple_FT * +find_tuple(Math_Expr_FT *tuple); + Rational is_constant_rational(Math_Expr_FT *expr, Function_Scope *scope, bool *found); diff --git a/src/llvm_jit.cpp b/src/llvm_jit.cpp index cbebca7..b4866c2 100644 --- a/src/llvm_jit.cpp +++ b/src/llvm_jit.cpp @@ -220,7 +220,6 @@ jit_compile_module(LLVM_Module_Data *data, std::string *output_string) { pb.crossRegisterProxies(lam, fam, cgam, mam); llvm::ModulePassManager mpm = pb.buildPerModuleDefaultPipeline(llvm::OptimizationLevel::O2); - mpm.run(*data->module, mam); if(output_string) { @@ -271,7 +270,13 @@ get_jitted_batch_function(const std::string &fun_name) { return nullptr; } -typedef Scope_Local_Vars Scope_Data; +struct +LLVM_Local_Var { + llvm::Value *val; + Math_Expr_FT *expr; +}; + +typedef Scope_Local_Vars Scope_Data; llvm::Value *build_expression_ir(Math_Expr_FT *expr, Scope_Data *scope, std::vector &args, LLVM_Module_Data *data); @@ -639,7 +644,7 @@ build_for_loop_ir(Math_Expr_FT *n, Math_Expr_FT *body, Scope_Data *loop_local, s // Create the iterator llvm::PHINode *iter = data->builder->CreatePHI(llvm::Type::getInt64Ty(*data->context), 2, "index"); iter->addIncoming(start_val, pre_header_block); - loop_local->values[0] = iter; + loop_local->values[0] = {iter, nullptr}; // Insert the loop body build_expression_ir(body, loop_local, args, data); @@ -743,18 +748,6 @@ build_external_computation_ir(Math_Expr_FT *expr, Scope_Data *locals, std::vecto } arguments.push_back(alloc); - /* - for(int idx = 0; idx < n_call_args; ++idx) { - auto alloc = data->builder->CreateAlloca(struct_ty); - auto val = data->builder->CreateStructGEP(struct_ty, alloc, 0); - data->builder->CreateStore(valptrs[idx], val); - auto stride = data->builder->CreateStructGEP(struct_ty, alloc, 1); - data->builder->CreateStore(strides[idx], stride); - auto count = data->builder->CreateStructGEP(struct_ty, alloc, 2); - data->builder->CreateStore(counts[idx], count); - arguments.push_back(alloc); - } - */ data->builder->CreateCall(external_fun, arguments); @@ -789,9 +782,9 @@ build_expression_ir(Math_Expr_FT *expr, Scope_Data *locals, std::vectorexprs) { if(sub_expr->expr_type == Math_Expr_Type::local_var) { auto local = static_cast(sub_expr); - if(local->is_used) { // Probably unnecessary since it should have been pruned away in that case. + if(local->is_used) { // Probably unnecessary since it should have been pruned away if unused result = build_expression_ir(sub_expr, &new_locals, args, data); - new_locals.values[local->id] = result; + new_locals.values[local->id] = {result, sub_expr->exprs[0]}; } } else result = build_expression_ir(sub_expr, &new_locals, args, data); @@ -813,7 +806,8 @@ build_expression_ir(Math_Expr_FT *expr, Scope_Data *locals, std::vector(expr); - auto local = find_local_var(locals, assign->local_var); + auto loc = find_local_var(locals, assign->local_var); + auto local = loc.val; if(!local->getType()->isPointerTy()) fatal_error(Mobius_Error::internal, "LLVM, trying to reassign a value to a local var that was not set up for it."); auto value = build_expression_ir(expr->exprs[0], locals, args, data); @@ -856,7 +850,8 @@ build_expression_ir(Math_Expr_FT *expr, Scope_Data *locals, std::vectorbuilder->CreateLoad(double_ty, result, "var"); } else if(ident->variable_type == Variable_Type::local) { - result = find_local_var(locals, ident->local_var); + auto local = find_local_var(locals, ident->local_var); + result = local.val; if(!result) fatal_error(Mobius_Error::internal, "A local var was not initialized in ir building."); if(result->getType()->isPointerTy()) { @@ -1012,6 +1007,39 @@ build_expression_ir(Math_Expr_FT *expr, Scope_Data *locals, std::vector elems; + std::vector elem_types; + for(auto ex : expr->exprs) { + elem_types.push_back(get_llvm_type(ex->value_type, data)); + elems.push_back(build_expression_ir(ex, locals, args, data)); + } + auto struct_ty = llvm::StructType::get(*data->context, elem_types); + auto strct = data->builder->CreateAlloca(struct_ty, nullptr, "tuple"); + for(int idx = 0; idx < elems.size(); ++idx) { + auto val = data->builder->CreateStructGEP(struct_ty, strct, idx); + data->builder->CreateStore(elems[idx], val); + } + return strct; + }; + + case Math_Expr_Type::access_tuple_element : { + auto access = static_cast(expr); + auto local = find_local_var(locals, access->tuple_id); + auto strct = local.val; + auto tuple = find_tuple(local.expr); + + std::vector elem_types; + for(auto ex : tuple->exprs) { + elem_types.push_back(get_llvm_type(ex->value_type, data)); + } + auto struct_ty = llvm::StructType::get(*data->context, elem_types); + + auto val = data->builder->CreateStructGEP(struct_ty, strct, access->element_index); + auto argty = get_llvm_type(access->value_type, data); + return data->builder->CreateLoad(argty, val, "tuple_elem"); + }; + case Math_Expr_Type::no_op : { //auto *fun = llvm::Intrinsic::getDeclaration(data->module.get(), llvm::Intrinsic::donothing, {}); //return data->builder->CreateCall(fun, {}, "no_op"); diff --git a/src/math_expr_types.incl b/src/math_expr_types.incl index 25c3778..1331fc3 100644 --- a/src/math_expr_types.incl +++ b/src/math_expr_types.incl @@ -13,6 +13,9 @@ ENUM_VALUE(state_var_assignment) ENUM_VALUE(derivative_assignment) ENUM_VALUE(external_computation) ENUM_VALUE(iterate) +ENUM_VALUE(tuple) +ENUM_VALUE(unpack_tuple) +ENUM_VALUE(access_tuple_element) ENUM_VALUE(no_op) ENUM_VALUE(regex_or_chain) ENUM_VALUE(regex_identifier) diff --git a/src/model_compilation.cpp b/src/model_compilation.cpp index 771df89..dfda63e 100644 --- a/src/model_compilation.cpp +++ b/src/model_compilation.cpp @@ -2000,6 +2000,7 @@ Model_Application::compile(bool store_code_strings) { for(auto &batch : batches) { Run_Batch new_batch; new_batch.run_code = generate_run_code(this, &batch, instructions, false); + if(is_valid(batch.solver)) { new_batch.solver_id = batch.solver;