Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TAREA5 CARLOS #58

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

TAREA5 CARLOS #58

wants to merge 29 commits into from

Conversation

carlosr47
Copy link
Contributor

primer commit

@lbenet
Copy link
Owner

lbenet commented Apr 24, 2017

No me queda claro por qué quieres tener un polinomio de orden 2, con 5 coeficientes.

En cuanto a las operaciones, te hago notar que la suma y la resta sólo están afectando a un coeficiente, y por lo mismo no están dando la respuesta correcta.

Por último, falta escribir mucho para saber qué estás pensando...

@lbenet
Copy link
Owner

lbenet commented Apr 25, 2017

Vale la pena cambiarle el nombre al módulo (AD), ya que ese nombre se usaba para los duales.

No es necesario que promuevas nada el arreglo de coeficientes y el grado, dado que coeff es un arreglo con un tipo T<:Number definido, y quieres que el grado sea un Int. De hecho, lo que haces en la función taylor(n,v) es lo que podría hacer la función Taylor(orden,coff).

Te hago notar que si quieres que haya compatibilidad entre la n y el vector de coeficientes, cuando no la hay es mejor que te de un error; además, la condición length(v) == n es incorrecta. Esta sutileza es importante, ya que el vector de coeficientes no corresponde con el grado.

En la celda [8], en la función xtaylor no es necesario que borres una entrada y luego la agregues; simplemente cámbiala. Esa función, por cierto, no define la variable independiente!

@lbenet
Copy link
Owner

lbenet commented Apr 27, 2017

Se obtiene el resultado esperado si haces: Taylor(2, [1,0,-1]) / Taylor(1, [1,1])? Esto debería ser equivalente a $ (1-x^2) / (1+x) $, y es una prueba forzosa que debes incluir en tus tests.

Falta comentar lo que estás haciendo. Por ejemplo, no me queda claro si es o no útil la función sumavector, entre otras...

@lbenet
Copy link
Owner

lbenet commented May 1, 2017

Algunos comentarios sobre lo que llevas de la tarea:

  • En la celda [2] es útil que lance un error si la longitud no es la que quieres. Sin embargo, el mensaje "función no válida" no es claro. Puedes poner "Parámetro n incompatible con la longitud de v" o algo así que le ayude al usuario a saber qué pasa. La función taylor la podrías poner como constructor interno.

  • Me parece que la función xTaylor pretende definir la variable independiente, o sea, respecto a la que derivas (x). Entonces, no debería haber término independiente.

  • Vale la pena hacerte notar que la función igualavector cambia a las funciones a y b (que pienso que son Taylor pero que no están especificadas como tal). Además, lo que devuelve esa función es a.orden, que no tiene mucho sentido.

  • Parecido al punto anterior, en sumavector y restavector, las funciones te regresan un vector; ¿es eso lo que quieres? Por otro lado, multvector te va a ir cerciendo; ¿es lo que quieres? Nota además que el resultado que da multvector(k,j) es incorrecto considerando que k corresponde al polinomio 1+x, y j al polinomio 1+2x+x^2 (celda [89]).

  • En el ejercicio 1 faltan varias situaciones, por ejemplo -taylor(2, [1,2]) o incluso +taylor(2, [1,2])

@lbenet
Copy link
Owner

lbenet commented May 9, 2017

Algunos comentarios sobre la última versión:

  • En el constructor interno, la comparación length(v)==n es incorrecta. Un polinomio de grado n tiene n+1 coeficientes, por lo que la longitud del vector debe ser comparada con n+1 y no con n. La sutileza de este comentario es importante, ya que influirá en otras funciones que implementaste, como es xTaylor.

  • En cuanto a lo que llevas de las operaciones, vas por buen camino. Sin embargo, en la celda [31] tienes (para la suma):

+(A::Taylor,B::Taylor)=Taylor(igualavector(A,B),sumavector(A,B))
+(A::Taylor,B::Taylor)=Taylor(igualavector(A,B),A.coff+B.coff)  #en caso de que el orden sea el mismo, la suma es directa

Lo que quiero enfatizar es que la segunda definición sobreescribe a la primera (la reemplaza!); si la quitas, el caso en que los órdenes sea el mismo estará cuebierto (en sumavector(A,B)). Esto se aplica tal cual a otras operaciones.

  • Hago la observación que no has implementado aún el caso a * b o a / b con a y b ambos Taylor.

@carlosr47
Copy link
Contributor Author

traté de usar algo relacionado con el constructor interno, un amigo me estuvo ayudando con esta primera parte, no obstante, me aparece el error "stackoverflow", ¿este error a que se debe?

@lbenet
Copy link
Owner

lbenet commented May 13, 2017

El problema del "Stack overflow" tiene que ver con la definición del método Taylor siguiente:

Taylor{T<:Number}(orden::Int,coff::Array{T,1})=Taylor(orden,coff)    #el caso general

Cuando tu escribes Taylor(2, [0,1,2]), Julia usa ese método (lado izquierdo del =) y te devuelve el lado derecho, que es lo mismo. Por eso el Stack overflow.

Si cambias este método por

Taylor{T<:Number}(orden::Int,coff::Array{T,1})=Taylor{T}(orden,coff)    #el caso general

las cosas funcionarán. La diferencia está en el {T} que hay del lado derecho del =; este método permite no escribirlo.

…aciones, correcciones propuestas por el profesor hechas, falta el producto y la division
@lbenet
Copy link
Owner

lbenet commented May 14, 2017

Algunos comentarios:

  • Es mejor que igualavector se llame igualavector!, ya que de hecho la función cambia (muta) a alguno de los parámetros. Eso se enfatiza con el !; es una convención. (Lo mismo se aplica a prod.) De hecho, esta función no es type-stable, pero eso lo dejaremos de lado...

  • En /, tu algoritmo no es adecuado; la idea no es mala, pero lo que haces no funciona. Por ejemplo, lo siguiente es una posibilidad para hacer la división de p = x^2, q = x, y no da el resultado esperado:

julia> using BD

julia> p = Taylor(3, [0,0,1,0])  # Corresponde a x^2 (orden 3)
BD.Taylor{Int64}(3,[0,0,1,0])

julia> q = Taylor(4, [0,1,0,0,0]) # Corresponde a x^2 (orden 4)
BD.Taylor{Int64}(4,[0,1,0,0,0])

julia> p/q
ERROR: división no válida ya que el resultado no es un polinomio de grado mayor a cero
 in /(::BD.Taylor{Int64}, ::BD.Taylor{Int64}) at /Users/benet/Documents/4-Clases/46_TemasSelectos/2017-2/tareas/Carlos/AutomTaylor.jl:82
  • En * hay un problema. Esto se ve en el resultado [17] de tu notebook. Otro ejemplo, para p y q definidos arriba, es:
julia> p*q
ERROR: BoundsError: attempt to access 4-element Array{Float64,1} at index [5]
 in *(::BD.Taylor{Int64}, ::BD.Taylor{Int64}) at /Users/benet/Documents/4-Clases/46_TemasSelectos/2017-2/tareas/Carlos/AutomTaylor.jl:66
  • En cuanto a ==, la pregunta es si Taylor(3,[1,2,3,4]) debe o no ser igual a Taylor(4,[1,2,3,4,0])). Si lo pienso como polinomios la respuesta es sí deben ser iguales; tu implementación dice que estos polinomios no son iguales.

  • Quiero que notes lo siguiente, que indica que hay problemas:

julia> Taylor(4,[1,2,3,4,0]) + Taylor(3,[1,2,3,4])
BD.Taylor{Int64}(4,[2,4,6,8,0])

julia> Taylor(3,[1,2,3,4]) + Taylor(4,[1,2,3,4,0])
ERROR: función no válida ya que el orden no coincide con el número de coeficientes
 in Type at /Users/benet/Documents/4-Clases/46_TemasSelectos/2017-2/tareas/Carlos/AutomTaylor.jl:15 [inlined]
 in BD.Taylor{T<:Number}(::Int64, ::Array{Int64,1}) at /Users/benet/Documents/4-Clases/46_TemasSelectos/2017-2/tareas/Carlos/AutomTaylor.jl:20
 in +(::BD.Taylor{Int64}, ::BD.Taylor{Int64}) at /Users/benet/Documents/4-Clases/46_TemasSelectos/2017-2/tareas/Carlos/AutomTaylor.jl:42
  • Por último, tus tests dan tres errores...

@lbenet
Copy link
Owner

lbenet commented May 16, 2017

Me parece que excepto por la convención de usar ! al final del nombre de la función, cuando la función muta alguno (o varios) de sus parámetros, mis demás comentarios no se han considerado.

Respecto a los últimos cambios, ¿qué quiere decir "gracias por el código de gitmora"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants