untoldwind / KontrolSystem2

Autopilot scripting system for KSP2
Other
54 stars 14 forks source link

Programming question #145

Closed PhilouDS closed 4 days ago

PhilouDS commented 3 months ago

I've been struggling with that for hour but I think someone with better programming skill has an (I hope easy) answer.

I'm creating an UI and I have a loop where I look at the engines. For each engine, I create a label line in the UI with the engine's name, its ISP and its FuelFlow (ISP and FuelFlow are irrelevant for my question... i'm just saying I have a string and two floats).

At the end of the loop : I want a list of the engines sorted by ISP (smaller to higher) and to have access to index of that engine in that list (index_for_isp). I want a list of the engines sorted by FuelFlow (smaller to higher) and to have access to index of that engine in that list (index_for_flow). Then, for each engine, I want the average of the two indices to get the engine with the best average average_index = (index_for_isp + index_for_flow)/2.

At the end of the script, I just want to print something like: "The best ISP is {value_of_isp} from engine {engine_name}" "The best fuel flow is {value_of_flow} from engine {engine_name}" "The best average engine is {engine_name}"

I try with a list of tuples [(engine_name, isp, flow), (...), ...] but I don't know how to sort using one element of a tuple. I try with a list of list [[engine_name, isp, flow], [...], ...] but it seems that all the element of one list must be of same type. And when you convert floats into strings, you can't sort them by numerical order.

Each time I think I have a solution, there is a problem. But I guess it can't be hard to do that... or is it?

Thanks for the help.

untoldwind commented 3 months ago

There is now the fairly new .sort_by on array.

In your case to sort const arr = [(engine_name, isp, flow), (...), ...]

PhilouDS commented 3 months ago

Thanks! I didn't know about tuple._1.

untoldwind commented 3 months ago

From a "clean code" perspective, the ._1, ._2 ... are kind of ugly because they are not expressive.

A cleaner alternative would be to use records like const arr = [(name: engine_name, isp: isp, flow: flow) ...]. Then the sort by can be written as .sort_by(fn(r) -> r.name), which is much easier to understand.

PhilouDS commented 3 months ago

From a "clean code" perspective, the ._1, ._2 ... are kind of ugly because they are not expressive.

A cleaner alternative would be to use records like const arr = [(name: engine_name, isp: isp, flow: flow) ...]. Then the sort by can be written as .sort_by(fn(r) -> r.name), which is much easier to understand.

Nice! Thank you!

PhilouDS commented 3 months ago

Hi! New question about something else. I start to have lot of global constants. If I put all of them in one my_constants.to2 file, is it possible to load all of them in use {...} from my_constants without enumerating all of them?

untoldwind commented 3 months ago
use * from my_constants

should do that, though it might fail if one name is not unique.

PhilouDS commented 3 months ago

How can I build Matrix n x n with only 1 in the diagonal and 0 elsewhere?

let line: int[] = []
let matrix: int[][] = []
for (i in 0..6) {line += [0]}
for (i in 0..6) {matrix += [line]}

for (i in 0..6) {matrix[i][i] = 1}

Why this code doesn't work?

I also try the following code for testing purpose, it didn't work either.

for (i in 0..6) {
    for (j in 0..6) {
        matrix[i][j] = i + j
    }
}

Also, I saw in to2 folder some script using ArrayBuilder. How does it work? There is nothing about this in the documentation and internet is not very helpful.

Thanks for your help.

untoldwind commented 3 months ago

Problem is that you are only creating one line array and then creating an array with all elements points to the same instance. Creating a matrix in a nested loop should work:

    let matrix : int[][] = []
    for(i in 0..6) {
        let line : int[] = []
        for (j in 0..6) {
          line += 0
        }
        matrix += line
    }

    for (i in 0..6) {matrix[i][i] = 1}

But this is a shortcoming since I wanted to avoid these under-the-hood kind of problems. I would say that in this scenario matrix += [line] should always create a clone/copy of the line array and not just pass the reference ... just have to figure out how to bake this into the language without clone/copy every array all the time

PhilouDS commented 3 months ago

Thx. I'll have a look at this later. I also need to program the product of two squared matrix. I let you know if I encounter other problem.

lefouvert commented 3 months ago

@PhilouDS Parenthesis about ArrayBuilder. As far, I've only identified 3 methods :

The purpose is to declare only one time contiguous memory in order to gain in performances. It's a standards in many libs (often seen with StringBuilder) since the loop way will declare contiguous memory as many times you loop to add an item to create a new array with your item added, and free the older one. You can imagine it's not really optimized. It's a way to garantee you your array is unmutable (once it's created, datas will not change inside) but with the performances of a mutable variable at the time of the build. As I didn't tried it, I dont know if ArrayBuilder is able to remove the excess of empty cells if any, at .result() time.

PhilouDS commented 3 months ago

It's a way to garantee you your array is unmutable (once it's created, datas will not change inside) but with the performances of a mutable variable at the time of the build.

Okay thanks. Isn't it a problem if you can't change the array?

@lefouvert I take the opportunity to ask you something about your code. In the boot file, you have CONSOLE.print_line((0..80).reduce("", fn(strike, c) -> strike + "-"))

From which language does .reduce come from? What is the strike parameter?

PhilouDS commented 3 months ago

Thx. I'll have a look at this later. I also need to program the product of two squared matrix. I let you know if I encounter other problem.

Ok. No problem with product :)

lefouvert commented 3 months ago

It's not a problem since you can build a new array with new values. It's used a lot in web app to not have to bury yourself with «the origin story» of the variable. It also more memory secure against leaks and out of ranges. Lot of old exploits use array to have an enter point in the memory and then glide away thru the memory to get data and modify data they shouldN'T. Intentionally or by accident.

Dunno where come from .reduce(). I know there is a .reduce() in Java, but it apply only on stream (an input with unknown size, think it like a keyboard entry. There is a char. And a bit after, an other char etc). C# have .Aggregate() which work the same, but on collection (kind of arrays, but each kind of collections have specific properties, as no dulicates, first in last out, etc) and in many other langages (javascript, erlang, rust etc) Each langage have his own interpretation of the .reduce(). The main purpose is to reduce (Oh! no way ?) a collection of data in something shorter, sort of usefull synthesis. For example, in java : streamExample.reduce(0, (subtotal, element) -> subtotal + element); have as first parameter something which combine with the function you will apply, DOES NOTHING (for a '+' function, 0 do nothing, for a '×' function, it's the 1 which does nothing, etc) . It's usefull for parrallelized operations. Then the second parameter is the definition of the function. As far as I know, you can take 2 or 3 parameters in a FUNCTION'S reduce. If it take 2 parameters, the first one is «the result», and the second one is «the item of the array you want to compute». If there is a third parameter, usualy, it's the combinator, it's a function (a pointer on function if we want an analogy with C language) which describe the operation you want to apply.

Let's back to to2. It work pretty much the same way than .Aggregate() in C#. Here, the first parameter is with what you want to initialize your result. The second parameter is still a function which describe how you want to combine the items in your array to have a reduced synthesis. (usefull to sum up all your items in a float array, for example) This function always have the same shape : fn(aggregation, item_to_process) -> something_the_type_of_aggregation. Here, the type of aggregation is defined by the type of your initialization of the aggregation, with the first parameter of the reduce.

About this row : CONSOLE.print_line((0..80).reduce("", fn(strike, c) -> strike + "-")) I process the range as an array of integers. Let's consider the array [0, 1, 2, ... , 78, 79] At first, .reduce() will create a new empty string "" Then it will browse this array from 0 to 79. Each time, it will execute the fn(strike, c) -> string function. It's a Lambda, (also known as Anonymous function), but think about it as any other function : you say 'Hey, it's a function, she's named 'JohnDoe', it take thoses parameters I want to call «foo» and «bar», and it does this and this, and return that' except you didn't named it. So .reduce() will exectue the function. It always expect the same kind of function : fn(name: type_of_first_parameter, name: type_of_an_item_in_the_array) -> type_of_first_parameter Here, I cheat, because my purpose is not to synthetize the data inside the array, but exploit the size of the array to build an unrelated new data. The new data is a string made of as many '-' char than they are items in the array. So for each int browsed in the array, the init value of the .reduce() will grow with one more '-'. It allow me to point out why unmutables array/strings etc are not an issue : each time we add a '-' to the strike string, it's not the same string with one more char, it's a completly new string containing the same datas as the previous one and a '-' more. And the returned value is also new, the initial array [0, 1, 2...] is still here and it insides didn't change.

Let's look at a less weird use of a .reduce() : ship::stage.to2

        const fuelFlowSum = self.engines.activation
            .map(fn(p) -> p.engine.value.max_fuel_flow * p.engine.value.max_thrust_output_vac / p.engine.value.current_engine_mode.max_thrust) // (fuel Flow * limiteur de poussée)
            .reduce(0.0, fn(sum, f) -> sum + f)

Let's skip the map part (we can talk about if you want, but for now, we look at the reduce) and we will consider an array of float feeding the reduce. Thoses floats represents the max fuel consumption for each engine parts in self.engines.activation array. So we have [3.26, 3.26, 4.32, ...] I want to know how much fuel I will burn at full thrust if all of thoses engines are ignited. The operation is really simple, I just have to add all the values in my array. I could do it this way :

let sum = 0.0
for( f in list_of_consumption_for_considered_engines )
    sum = sum + f // or sum += f

It's heavy and boring, even if it's usefull. Here come the reduce :

list_of_consumption_for_considered_engines.reduce(0.0, fn(sum, f) -> sum + f)

So it will declare a new variable sum, with the initial value 0.0, and will apply this function for each iteration of items in the array return sum + f. Function have sum and the current item f as parameters. At each iteration, the variable initialy declared as sum will be replaced by the result of the function. At the end, it will return sum which should match with the sum of all the items in list_of_consumption_for_considered_engines. That is an usual way to use .reduce()

Consider the fact than reduce is usually a terminal function, since it should return something you caN'T use with arrays' methods. You can chain array's methods since most of them returns arrays, so you can arr_example.filter(things).map(things).reduce(things) but in most case, arr_example.reduce(things).filter(things) is probably a big mistake.

Don't hesitate to ask more question, on this topics (.reduce()) if I'm not enough clear, or others (map() ? .filter() ?). I'm sure untoldwind will be able to correct me if i'm too imprecise or blurry (or wrong !), but I don't think I'm so far of the thruth.

Edit : some words forgetten put in CAPS

untoldwind commented 3 months ago

In the next version there should be some array cloning happening in case a read-only array is assigned to a read-write variable, i.e. the original code to create a matrix should then work as intended.

As for the ArrayBuilder ... that is just an optimization to avoid unnecessary memory allocations. It only becomes relevant when creating large array (>1000 or so), for smaller arrays that should be almost no difference.

There is though another (more functional) way to create an identity matrix of arbitrary dimensions:

sync fn create_ident_matrix(rows: int, cols: int) -> float[][] = 
    (0..rows).map(fn(row) -> (0..cols).map(fn(col) -> 
        if(row == col) 1.0 else 0.0
    ))
PhilouDS commented 3 months ago
sync fn create_ident_matrix(rows: int, cols: int) -> float[][] = 
    (0..rows).map(fn(row) -> (0..cols).map(fn(col) -> 
        if(row == col) 1.0 else 0.0
    ))

That's pretty nice!

PhilouDS commented 3 months ago

Hello, What wuold be the best use for each vesslel_autopilot?

Until now, I was using target_orientation but I figured out that I need lock_direction to be able to roll.

lefouvert commented 3 months ago

I didn't realized. Don't know if we really can define one or other as «the best», it depend of the context, but I'll try this nice lock_direction right now. It could be the solution of some of my issues :) Thank you !

Edit : for my use, I would say target_orientation for on the fly steering, and lock_direction for static use, as docking, parking etc (not moving, or not moving fast operation, which could need roll). global or not, still for me, is matter of which kind of vector or direction I have the most at computation time. Caution, global expires fast : it happen to me than in PAUSED GAME, values and equalities change with globals.

PhilouDS commented 3 months ago

@lefouvert Until now, how did you get your vessel rolling without lock_direction?

You might be right about static use: for my gravity turn, when I use target_orientation, I can pitch but not roll. With lock_direction, I can roll but not pitch!

My gravity turn is simple: I want to follow the prograde (surface or orbital) vector and to have a roll angle equal to my azimuth.

I tried to use manage_steering without success. But it does'nt really matter since I'm using while loops during my GT.

untoldwind commented 3 months ago

Some stuff about the SAS system I have not figured out yet (and there might be changes to it in the upcoming game releases). What works is AutopilotMode.Autopilot (maybe AutopilotMode.Navigation as well) with a target_orientation (or the global-variant).

.lock_direction was/is supposed to also control the rotation of the vessel, but I had some mixed results with that.

lefouvert commented 3 months ago

@PhilouDS Most of the time, I didn't managed roll. The only times where roll matters, I use std::control::steering:: control_steering and I have mixed result too : When command module is in default orientation at the build time, everything is fine, else, it don't work.

manage_steering is hard to use because you have to update your controls yourself, considere inertia etc. If think it require a PID-loop to be usefull, but as I have work on some years ago, it's a pain : easy to code, hard to tune, and very specialized for each ship. As my mecanics knowledge are limited and very old, build an autotuning PID-loop in function of mass, inertia, reaction time etc is above my skills.

As I didn't start thinking about planes (it's my last priority ^^), all my flight are managed by target_orientation and result are really good. But now I have a maneuver where roll matter, and I'm pretty stuck : control_steering is capricious, manage_steering is a pain, and for me, autopilot.lock_direction does absolutly nothing.

PhilouDS commented 3 months ago

I changed the while loops of my GT and added those two lines to manage both roll and pitch/yaw. This does what I want.

vessel.autopilot.lock_direction = vessel.heading_direction(azimuth, angle_above_horizon, azimuth)
vessel.autopilot.target_orientation = vessel.heading_direction(azimuth, angle_above_horizon, azimuth).vector
lefouvert commented 3 months ago

vessel.autopilot.target_orientation = vessel.heading_direction(azimuth, angle_above_horizon, azimuth).vector This is currently what I use. The maneuver I want to do which implie a roll is, as far as I know, not doeable with heading_direction. (It's the solar_panels orientation). Howerver, I don't understand why when I set up any Direction in lock_direction, nothing happen...

vessel.autopilot.enabled = true
vessel.autopilot.mode = AutopilotMode.Autopilot
vessel.autopilot.lock_direction = ksp::math::euler(90, 0, 90)
// And then... nothing.

Let me know when it's available on your git, I'll take a look ;)

Uh oh, and, don't have many expectation about std::control::steering:: control_steering, even if Untoldwinds is digging it to found what's wrong (thanks to him), because I think this tool dont meet your need. In the fact, it make a two time maneuver : it set the pitch/yaw, and then, correct the roll. As I think you are working on planes, (I'm curious too see it, it should be interesting), this kind of attitude correction doesn't match your need, I think.

PhilouDS commented 3 months ago

Let me know when it's available on your git, I'll take a look ;)

This is my gravity turn function (I'm playing with a 2.5x scaled system, that's why my space altitude is 86000m):

pub fn gravity_turn (craft: Vessel, target_apo: float, pitch_angle: float, azimuth: float, jettison_fairing: bool = false) -> Unit = {
  CONSOLE.clear()

  let from_north = clamp_degrees360(azimuth)

  print_title("GRAVITY TURN IN PROGRESS", 0, false)
  const this_row = CONSOLE.cursor_row
  let no_more_fairing = false
  let in_space: bool = false

  let angle_update = 0.0
  let t_gt = current_time()
  while(craft.facing.vector.angle_to(craft.heading_direction(from_north, pitch_angle, 0).vector) > 0.5) {
    angle_update = (90 - pitch_angle)*(current_time()-t_gt)/3.0
    craft.autopilot.lock_direction = craft.heading_direction(from_north, 90-angle_update, azimuth)
    craft.autopilot.target_orientation = craft.heading_direction(from_north, 90-angle_update, azimuth).vector
    check_staging(craft)
    yield()
  }

  while(craft.facing.vector.angle_to(craft.surface_velocity) > 0.5) {
    check_staging(craft)
    yield()
  }

  let angle_above_horizon = 90 - craft.up.angle_to(craft.surface_velocity)

  while (craft.altitude_sealevel < 36000) {
    check_staging(craft)
    angle_above_horizon = 90 - craft.up.angle_to(craft.surface_velocity)
    craft.autopilot.lock_direction = craft.heading_direction(from_north, angle_above_horizon, azimuth)
    craft.autopilot.target_orientation = craft.heading_direction(from_north, angle_above_horizon, azimuth).vector
    yield()
  }

  while (craft.orbit.apoapsis.value < target_apo) {
    check_staging(craft)
    angle_above_horizon = 90 - craft.up.angle_to(craft.orbit.prograde(current_time()))
    craft.autopilot.lock_direction = craft.heading_direction(from_north, angle_above_horizon, azimuth)
    craft.autopilot.target_orientation = craft.heading_direction(from_north, angle_above_horizon, azimuth).vector
    if (craft.orbit.apoapsis.value > 0.95 * target_apo) {craft.set_throttle(max(0.1, 30/craft.available_thrust))}
    if (craft.altitude_sealevel > 86000 && jettison_fairing && !no_more_fairing) {
      craft.parts.filter_map(fn(p) -> p.fairing)[0].jettison()
      no_more_fairing = true
      CONSOLE.clear()
      in_space = true
      print_title("IN SPACE", 0)
      sleep(0.5)
    }
    yield()
  }

  yield()
  craft.set_throttle(0)
  CONSOLE.clear()
  print_title("GRAVITY TURN DONE", 0)
  sleep(1)

  if (!in_space) {
    while(craft.altitude_sealevel < 86000) {
      angle_above_horizon = 90 - craft.up.angle_to(craft.orbit.prograde(current_time()))
      craft.autopilot.lock_direction = craft.heading_direction(from_north, angle_above_horizon, azimuth)
      craft.autopilot.target_orientation = craft.heading_direction(from_north, angle_above_horizon, azimuth).vector
      yield()
    }
    yield()
    CONSOLE.clear()
    print_title("IN SPACE", 0)
    if (!no_more_fairing && jettison_fairing) {craft.parts.filter_map(fn(p) -> p.fairing)[0].jettison()}
  }
  craft.autopilot.lock_direction = craft.heading_direction(from_north, angle_above_horizon, 0)
  sleep(1)
}

Uh oh, and, don't have many expectation about std::control::steering:: control_steering, even if Untoldwinds is digging it to found what's wrong (thanks to him), because I think this tool dont meet your need. In the fact, it make a two time maneuver : it set the pitch/yaw, and then, correct the roll.

I don't have any expectation, I never use any script from the to2 folder. I look at same sometimes to try to understand things (and I usually don't). In fact, I always delete all the contents of this folder to only see my scripts in the console.

As I think you are working on planes, (I'm curious too see it, it should be interesting), this kind of attitude correction doesn't match your need, I think.

I'm not working on planes for now. I was only sending rocket to orbital orbits to do some orbital survey. For that, I use an azimuth of ~ -6.7° and I wanted my rocket to roll to that angle. Btw, I don't know if I need to use -6.7° or 353.3°. It seems that the yaw uses 353.3° but the roll can use -6.7°.

lefouvert commented 3 months ago

I have a lots of comments to do. Somes questions too. Do you think it could be valuable we share a way to contact eachself to chitchat about all thoses ? Obviously, for usefull conclusion, we could report them here in order to make them available. Discord maybe ?

PhilouDS commented 3 months ago

Hello @untoldwind I am in an orbit around the mun with an inclination of 20°. A waypoint is at a latitude of 12°. Does a function already exist to know the two points of my orbit where my vessel above this latitude or should I compute it myself?

untoldwind commented 3 months ago

Not as part of the API, but there is the vacuum landing script in std:: https://github.com/untoldwind/KontrolSystem2/blob/master/KSP2Runtime/to2/std/land/vac.to2

This function is roughly doing the calculation you are describing: https://github.com/untoldwind/KontrolSystem2/blob/f6e40766c62ca18800834f1916a87d09ff35a560/KSP2Runtime/to2/std/land/vac.to2#L68

General idea is:

... not sure if this is helpful. I hacked this together quiet a while ago ;)

PhilouDS commented 3 months ago

Thanks :)

PhilouDS commented 2 months ago

Hello, I want to define myself the normal vector of an orbit like this:

pub sync fn norm_obt (lan: float, inc: float) -> Vec3 = {
  const a = sin_deg(lan) * sin_deg(inc)
  const b = -cos_deg(lan) * sin_deg(inc)
  const c = cos_deg(inc)

  vec3(a, b, c)
}

In which frame is it created?

If I want the normal vector (of type Vec3) of an orbit around a body body, should I return vec3(a, b, c).to_global(body.body_frame).to_local(body.body_frame)? Is it how orbit.orbit_normal is defined? I didn't know where to find this definition in the source code.

Thanks for the help.

lefouvert commented 2 months ago

Since longitude of ascending node and inclination are referenced from body, I would guess it's already in body.body_frame. vec3(a, b, c).to_global(body.body_frame).to_local(body_frame) is an useless redundancy. I would guess orbit.orbit_normal is in body.celestial_frame since in some of my code, I use it and all other vectors are in body.celestial_frame and all works fine. (Anyway, it seems body.celestial_frame is the default frame when no mention are done) (An easy way to know is to compute the dot product of vec3(a, b, c).normalized with orbit.orbit_normal.normalized, if it doesn't say the frame, at least, if the result is 1 or -1, you know it's in the same frame. I guess in this case it will not be 1.)

PhilouDS commented 2 months ago

Thanks. I really appreciate your answer but I'm gonna wait for Untoldwind's answer to have more than just a guess.

Also, I want to compute different orbit intersections and work with my vessel's position. Thanks to you answer, I realize than I probably should work in celestial_frame.

lefouvert commented 2 months ago

I understand. However, I would like to amend my answer with some correction. Since LAN is NOT changing in the time, it's referenced on body in a no-moving frame. body_celestal_frame become more logical as guess, if your computation match any frame.

untoldwind commented 2 months ago

By itself a vec3(x, y, z) (i.e. a Vec3) is not associated in to any reference frame ... it is just a plain old 3-dimensional vector.

It becomes associated with to a reference frame via v.to_global(frame), this in itself does not do any kind of transformation, it just says: "Please considered v to be coordinates in frame". An actual transformation can be done via v.to_global(frame).to_local(other_frame), i.e. what do the coordinates v in frame look like in other_frame.

In case of the orbit normal: All orbit calculations are done in orbit.reference_frame, which is somewhat tricky:

In the source code: I just noticed that there is no coordinate independent version of orbit_normal, the "regular" on is implemented here: https://github.com/untoldwind/KontrolSystem2/blob/704626a5bf8decde853e1e45e04ec571e0ba0cf0/KSP2Runtime/KSPOrbit/OrbitWrapper.cs#L44 ... which uses a method from the PatchedConicsOrbit class of the game ... which is also notorious in swapping the YZ coordinates (I am pretty certain that this is a left-over from KSP1)

untoldwind commented 2 months ago

The orbit.global_orbit_normal should be part of 0.5.8.3

github-actions[bot] commented 2 weeks ago

This issue is stale because it has been open for 60 days with no activity.

github-actions[bot] commented 4 days ago

This issue was closed because it has been inactive for 14 days since being marked as stale.